Merge pull request #35 from bryanburgers/more-structured-comments

Add `Approved` and `Delegated` structured comments

Author: pietroalbini

homu/comments.py

@@ -18,6 +18,59 @@ def jsonify(self):
         return json.dumps(out, separators=(',', ':'))
 
 
+class Approved(Comment):
+    def __init__(self, bot=None, **args):
+        # Because homu needs to leave a comment for itself to kick off a build,
+        # we need to know the correct botname to use. However, we don't want to
+        # save that botname in our state JSON. So we need a custom constructor
+        # to grab the botname and delegate the rest of the keyword args to the
+        # Comment constructor.
+        super().__init__(**args)
+        self.bot = bot
+
+    params = ["sha", "approver"]
+
+    def render(self):
+        # The comment here is required because Homu wants a full, unambiguous,
+        # pinned commit hash to kick off the build, and this note-to-self is
+        # how it gets it. This is to safeguard against situations where Homu
+        # reloads and another commit has been pushed since the approval.
+        message = ":pushpin: Commit {sha} has been " + \
+            "approved by `{approver}`\n\n" + \
+            "<!-- @{bot} r={approver} {sha} -->"
+        return message.format(
+            sha=self.sha,
+            approver=self.approver,
+            bot=self.bot
+        )
+
+
+class ApprovalIgnoredWip(Comment):
+    def __init__(self, wip_keyword=None, **args):
+        # We want to use the wip keyword in the message, but not in the json
+        # blob.
+        super().__init__(**args)
+        self.wip_keyword = wip_keyword
+
+    params = ["sha"]
+
+    def render(self):
+        message = ':clipboard:' + \
+            ' Looks like this PR is still in progress,' + \
+            ' ignoring approval.\n\n' + \
+            'Hint: Remove **{wip_keyword}** from this PR\'s title when' + \
+            ' it is ready for review.'
+        return message.format(wip_keyword=self.wip_keyword)
+
+
+class Delegated(Comment):
+    params = ["delegator", "delegate"]
+
+    def render(self):
+        message = ':v: @{} can now approve this pull request'
+        return message.format(self.delegate)
+
+
 class BuildStarted(Comment):
     params = ["head_sha", "merge_sha"]
 

homu/main.py

@@ -497,13 +497,10 @@ def parse_commands(body, username, repo_label, repo_cfg, state, my_username,
             for wip_kw in ['WIP', 'TODO', '[WIP]', '[TODO]', '[DO NOT MERGE]']:
                 if state.title.upper().startswith(wip_kw):
                     if realtime:
-                        state.add_comment((
-                            ':clipboard:'
-                            ' Looks like this PR is still in progress,'
-                            ' ignoring approval.\n\n'
-                            'Hint: Remove **{}** from this PR\'s title when'
-                            ' it is ready for review.'
-                        ).format(wip_kw))
+                        state.add_comment(comments.ApprovalIgnoredWip(
+                            sha=state.head_sha,
+                            wip_keyword=wip_kw,
+                        ))
                     is_wip = True
                     break
             if is_wip:
@@ -565,14 +562,10 @@ def parse_commands(body, username, repo_label, repo_cfg, state, my_username,
                         .format(msg, state.head_sha)
                     )
                 else:
-                    state.add_comment(
-                        ':pushpin: Commit {} has been approved by `{}`\n\n<!-- @{} r={} {} -->'  # noqa
-                        .format(
-                            state.head_sha,
-                            approver,
-                            my_username,
-                            approver,
-                            state.head_sha,
+                    state.add_comment(comments.Approved(
+                        sha=state.head_sha,
+                        approver=approver,
+                        bot=my_username,
                     ))
                     treeclosed = state.blocked_by_closed_tree()
                     if treeclosed:
@@ -620,10 +613,10 @@ def parse_commands(body, username, repo_label, repo_cfg, state, my_username,
             state.save()
 
             if realtime:
-                state.add_comment(
-                    ':v: @{} can now approve this pull request'
-                    .format(state.delegate)
-                )
+                state.add_comment(comments.Delegated(
+                    delegator=username,
+                    delegate=state.delegate
+                ))
 
         elif word == 'delegate-':
             # TODO: why is this a TRY?
@@ -640,10 +633,10 @@ def parse_commands(body, username, repo_label, repo_cfg, state, my_username,
             state.save()
 
             if realtime:
-                state.add_comment(
-                    ':v: @{} can now approve this pull request'
-                    .format(state.delegate)
-                )
+                state.add_comment(comments.Delegated(
+                    delegator=username,
+                    delegate=state.delegate
+                ))
 
         elif word == 'retry' and realtime:
             if not _try_auth_verified():