[llvm] [GitHub][workflows] Ask reviewers to merge new contributor's PRs (PR #80659)

David Spickett via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 5 02:55:00 PST 2024


https://github.com/DavidSpickett created https://github.com/llvm/llvm-project/pull/80659

This is based on GitHub's example for this use case: https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#running-a-workflow-when-a-pull-request-is-approved

When a review is submitted we check:
* If it's an approval.
* Whether it's on a PR from a new contributor (using the same logic as `new-prs.yml`).

Then if we haven't already left a comment, add one to tell the author that they can have the PR merged by a reviewer. PRs can get multiple approvals, so we'll look for another magic HTML comment to tell if we've already done this.

The comment doesn't ask the reviewers to merge it right away, just on the chance that the author still had things to do. As we don't have a norm of merging as soon as there is an approval, so doing that without asking might be surprising.

It also notes that if we need multiple approvals to wait for those. Though in that situation I don't think GitHub will enable the merge button until they've all approved anyway.

In future we can make this check more clever, perhaps by using a GitHub API to check all users and see if they have permissions. It's limited to new contributor PRs for now since we know for sure they won't have permissions.

With this change, new contributor PRs will get:
* A welcome comment on submission.
* This merge on behalf comment on first approval.
* A post merge comment about what to expect from the build bots.

>From a1c80783449e13515749b8da23e3057765cf77ef Mon Sep 17 00:00:00 2001
From: David Spickett <david.spickett at linaro.org>
Date: Fri, 2 Feb 2024 16:15:48 +0000
Subject: [PATCH] [GitHub][workflows] Ask reviewers to merge new contributor's
 PRs

This is based on GitHub's example for this use case:
https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#running-a-workflow-when-a-pull-request-is-approved

When a review is submitted we check:
* If it's an approval.
* Whether it's on a PR from a new contributor (using the same logic as `new-prs.yml`).

Then if we haven't already left a comment, add one to tell the author
that they can have the PR merged by a reviewer. PRs can get multiple
approvals, so we'll look for another magic HTML comment to tell if
we've already done this.

The comment doesn't ask the reviewers to merge it right away, just on
the chance that the author still had things to do. As we don't have
a norm of merging as soon as there is an approval, so doing that without
asking might be surprising.

It also notes that if we need multiple approvals to wait for those.
Though in that situation I don't think GitHub will enable the merge
button until they've all approved anyway.

In future we can make this check more clever, perhaps by using a GitHub
API to check all users and see if they have permissions. It's limited
to new contributor PRs for now since we know for sure they won't have
permissions.

With this change, new contributor PRs will get:
* A welcome comment on submission.
* This merge on behalf comment on first approval.
* A post merge comment about what to expect from the build bots.
---
 .github/workflows/approved-prs.yml  | 47 +++++++++++++++++++++++++++++
 llvm/utils/git/github-automation.py | 39 ++++++++++++++++++++++++
 2 files changed, 86 insertions(+)
 create mode 100644 .github/workflows/approved-prs.yml

diff --git a/.github/workflows/approved-prs.yml b/.github/workflows/approved-prs.yml
new file mode 100644
index 0000000000000..b8f911c87d9fc
--- /dev/null
+++ b/.github/workflows/approved-prs.yml
@@ -0,0 +1,47 @@
+name: "Prompt reviewers to merge PRs on behalf of new contributors."
+
+permissions:
+  contents: read
+
+on:
+  pull_request_review:
+    types:
+      - submitted
+
+jobs:
+  merge_on_behalf_information_comment:
+    runs-on: ubuntu-latest
+    permissions:
+      pull-requests: write
+    # If this PR was approved, and is authored by a new contributor. New
+    # contributor statuses don't work as expected, so we check that it is not
+    # any other status instead.
+    # (see https://github.com/orgs/community/discussions/78038)
+    if: >-
+      (github.repository == 'llvm/llvm-project') &&
+      (github.event.review.state == 'APPROVED') &&
+      (github.event.pull_request.author_association != 'COLLABORATOR') &&
+      (github.event.pull_request.author_association != 'CONTRIBUTOR') &&
+      (github.event.pull_request.author_association != 'MANNEQUIN') &&
+      (github.event.pull_request.author_association != 'MEMBER') &&
+      (github.event.pull_request.author_association != 'OWNER')
+    steps:
+      - name: Checkout Automation Script
+        uses: actions/checkout at v4
+        with:
+          sparse-checkout: llvm/utils/git/
+          ref: main
+
+      - name: Setup Automation Script
+        working-directory: ./llvm/utils/git/
+        run: |
+          pip install -r requirements.txt
+
+      - name: Add Merge On Behalf Information Comment
+        working-directory: ./llvm/utils/git/
+        run: |
+          python3 ./github-automation.py \
+            --token '${{ secrets.GITHUB_TOKEN }}' \
+            pr-merge-on-behalf-information \
+            --issue-number "${{ github.event.pull_request.number }}" \
+            --author "${{ github.event.pull_request.user.login }}"
diff --git a/llvm/utils/git/github-automation.py b/llvm/utils/git/github-automation.py
index 04698cacbff92..a76fe4f01c0c3 100755
--- a/llvm/utils/git/github-automation.py
+++ b/llvm/utils/git/github-automation.py
@@ -298,6 +298,32 @@ def run(self) -> bool:
         return True
 
 
+class PRMergeOnBehalfInformation:
+    COMMENT_TAG = "<!--LLVM MERGE ON BEHALF INFORMATION COMMENT-->\n"
+
+    def __init__(self, token: str, repo: str, pr_number: int, author: str):
+        repo = github.Github(token).get_repo(repo)
+        self.pr = repo.get_issue(pr_number).as_pull_request()
+        self.author = author
+
+    def run(self) -> bool:
+        # A review can be approved more than once, only comment the first time.
+        for comment in self.pr.as_issue().get_comments():
+            if self.COMMENT_TAG in comment.body:
+                return
+
+        # This text is using Markdown formatting.
+        comment = f"""\
+{self.COMMENT_TAG}
+@{self.author}, as a new contributor you do not have permissions to merge your own PRs yet.
+Please let us know when you are happy for this to be merged, and one of the reviewers can merge it on your behalf.
+
+(if many approvals are required, please wait until everyone has approved before merging)
+"""
+        self.pr.as_issue().create_comment(comment)
+        return True
+
+
 def setup_llvmbot_git(git_dir="."):
     """
     Configure the git repo in `git_dir` with the llvmbot account so
@@ -647,6 +673,14 @@ def execute_command(self) -> bool:
 pr_buildbot_information_parser.add_argument("--issue-number", type=int, required=True)
 pr_buildbot_information_parser.add_argument("--author", type=str, required=True)
 
+pr_merge_on_behalf_information_parser = subparsers.add_parser(
+    "pr-merge-on-behalf-information"
+)
+pr_merge_on_behalf_information_parser.add_argument(
+    "--issue-number", type=int, required=True
+)
+pr_merge_on_behalf_information_parser.add_argument("--author", type=str, required=True)
+
 release_workflow_parser = subparsers.add_parser("release-workflow")
 release_workflow_parser.add_argument(
     "--llvm-project-dir",
@@ -700,6 +734,11 @@ def execute_command(self) -> bool:
         args.token, args.repo, args.issue_number, args.author
     )
     pr_buildbot_information.run()
+elif args.command == "pr-merge-on-behalf-information":
+    pr_merge_on_behalf_information = PRMergeOnBehalfInformation(
+        args.token, args.repo, args.issue_number, args.author
+    )
+    pr_merge_on_behalf_information.run()
 elif args.command == "release-workflow":
     release_workflow = ReleaseWorkflow(
         args.token,



More information about the llvm-commits mailing list