Skip to content

Commit 889aedf

Browse files
committed
Add Approved and Delegated structured comments
Sometimes, Homu will leave an unstructured comment on a pull request, like: :emoji: Something happened! Other times, Homu will leave a structured comment, which is some text followed by a hidden JSON blob, like: :emoji: Something happened! <!-- homu: {"type":"SomethingHappened","arg1":"value"} --> This commit converts the "approved", "delegated", and "approval ignored because the pull request is a WIP" messages from unstructured to structured. Having more of our comments be structured may help us perform better startup synchronization in the future, because we can better inspect what Homu thought its state was before it was restarted.
1 parent 0c36bc4 commit 889aedf

File tree

2 files changed

+68
-23
lines changed

2 files changed

+68
-23
lines changed

homu/comments.py

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,58 @@ def jsonify(self):
1818
return json.dumps(out, separators=(',', ':'))
1919

2020

21+
class Approved(Comment):
22+
def __init__(self, bot=None, **args):
23+
# Because homu needs to leave a comment for itself to kick off a build,
24+
# we need to know the correct botname to use. However, we don't want to
25+
# save that botname in our state JSON. So we need a custom constructor
26+
# to grab the botname and delegate the rest of the keyword args to the
27+
# Comment constructor.
28+
super().__init__(**args)
29+
self.bot = bot
30+
31+
params = ["sha", "approver"]
32+
33+
def render(self):
34+
# The comment here is apparently required because Homu seems to use
35+
# this note-to-self comment to kick off the build. Presumably because
36+
# it contains the full commit hash?
37+
message = ":pushpin: Commit {sha} has been " + \
38+
"approved by `{approver}`\n\n" + \
39+
"<!-- @{bot} r={approver} {sha} -->"
40+
return message.format(
41+
sha=self.sha,
42+
approver=self.approver,
43+
bot=self.bot
44+
)
45+
46+
47+
class ApprovalIgnoredWip(Comment):
48+
def __init__(self, wip_keyword=None, **args):
49+
# We want to use the wip keyword in the message, but not in the json
50+
# blob.
51+
super().__init__(**args)
52+
self.wip_keyword = wip_keyword
53+
54+
params = ["sha"]
55+
56+
def render(self):
57+
message = ':clipboard:' + \
58+
' Looks like this PR is still in progress,' + \
59+
' ignoring approval.\n\n' + \
60+
'Hint: Remove **{wip_keyword}** from this PR\'s title when' + \
61+
' it is ready for review.'
62+
return message.format(wip_keyword=self.wip_keyword)
63+
64+
65+
class Delegated(Comment):
66+
params = ["delegator", "delegate"]
67+
68+
def render(self):
69+
message = ':v: @{} can now approve this pull request'
70+
return message.format(self.delegate)
71+
72+
2173
class BuildStarted(Comment):
2274
params = ["head_sha", "merge_sha"]
2375

homu/main.py

Lines changed: 16 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -497,13 +497,10 @@ def parse_commands(body, username, repo_label, repo_cfg, state, my_username,
497497
for wip_kw in ['WIP', 'TODO', '[WIP]', '[TODO]', '[DO NOT MERGE]']:
498498
if state.title.upper().startswith(wip_kw):
499499
if realtime:
500-
state.add_comment((
501-
':clipboard:'
502-
' Looks like this PR is still in progress,'
503-
' ignoring approval.\n\n'
504-
'Hint: Remove **{}** from this PR\'s title when'
505-
' it is ready for review.'
506-
).format(wip_kw))
500+
state.add_comment(comments.ApprovalIgnoredWip(
501+
sha=state.head_sha,
502+
wip_keyword=wip_kw,
503+
))
507504
is_wip = True
508505
break
509506
if is_wip:
@@ -565,14 +562,10 @@ def parse_commands(body, username, repo_label, repo_cfg, state, my_username,
565562
.format(msg, state.head_sha)
566563
)
567564
else:
568-
state.add_comment(
569-
':pushpin: Commit {} has been approved by `{}`\n\n<!-- @{} r={} {} -->' # noqa
570-
.format(
571-
state.head_sha,
572-
approver,
573-
my_username,
574-
approver,
575-
state.head_sha,
565+
state.add_comment(comments.Approved(
566+
sha=state.head_sha,
567+
approver=approver,
568+
bot=my_username,
576569
))
577570
treeclosed = state.blocked_by_closed_tree()
578571
if treeclosed:
@@ -620,10 +613,10 @@ def parse_commands(body, username, repo_label, repo_cfg, state, my_username,
620613
state.save()
621614

622615
if realtime:
623-
state.add_comment(
624-
':v: @{} can now approve this pull request'
625-
.format(state.delegate)
626-
)
616+
state.add_comment(comments.Delegated(
617+
delegator=username,
618+
delegate=state.delegate
619+
))
627620

628621
elif word == 'delegate-':
629622
# TODO: why is this a TRY?
@@ -640,10 +633,10 @@ def parse_commands(body, username, repo_label, repo_cfg, state, my_username,
640633
state.save()
641634

642635
if realtime:
643-
state.add_comment(
644-
':v: @{} can now approve this pull request'
645-
.format(state.delegate)
646-
)
636+
state.add_comment(comments.Delegated(
637+
delegator=username,
638+
delegate=state.delegate
639+
))
647640

648641
elif word == 'retry' and realtime:
649642
if not _try_auth_verified():

0 commit comments

Comments
 (0)