[llvm] [GitHub][workflows] Ask reviewers to merge PRs when author can not (PR #81142)
David Spickett via llvm-commits
llvm-commits at lists.llvm.org
Mon Feb 12 07:55:52 PST 2024
https://github.com/DavidSpickett updated https://github.com/llvm/llvm-project/pull/81142
>From ede1f5f2cdd3ef9ba6a62502277abe2890eeba42 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 1/4] [GitHub][workflows] Ask reviewers to merge PRs when
author can not
This uses https://pygithub.readthedocs.io/en/stable/github_objects/Repository.html?highlight=get_collaborator_permission#github.Repository.Repository.get_collaborator_permission.
Which does https://docs.github.com/en/rest/collaborators/collaborators?apiVersion=2022-11-28#get-repository-permissions-for-a-user and returns the top level "permission" key.
This is less detailed than the user/permissions key but should be fine for this
use case.
When a review is submitted we check:
* If it's an approval.
* Whether we have already left a merge on behalf comment (by looking for a hidden HTML comment).
* Whether the author has permissions to merge their own PR.
The comment doesn't ask the reviewers to merge it right away, just in
case 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.
---
.github/workflows/approved-prs.yml | 38 +++++++++++++++++++++
llvm/utils/git/github-automation.py | 51 +++++++++++++++++++++++++++++
2 files changed, 89 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 00000000000000..6a037f7aead0e9
--- /dev/null
+++ b/.github/workflows/approved-prs.yml
@@ -0,0 +1,38 @@
+name: "Prompt reviewers to merge PRs on behalf of authors"
+
+permissions:
+ contents: read
+
+on:
+ pull_request_review:
+ types:
+ - submitted
+
+jobs:
+ merge_on_behalf_information_comment:
+ runs-on: ubuntu-latest
+ permissions:
+ pull-requests: write
+ if: >-
+ (github.repository == 'llvm/llvm-project') &&
+ (github.event.review.state == 'APPROVED')
+ 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 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 04698cacbff929..c7483d4e6863d3 100755
--- a/llvm/utils/git/github-automation.py
+++ b/llvm/utils/git/github-automation.py
@@ -298,6 +298,44 @@ 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):
+ self.repo = github.Github(token).get_repo(repo)
+ self.pr = self.repo.get_issue(pr_number).as_pull_request()
+ self.author = author
+
+ def run(self) -> bool:
+ # Check this first because it only costs 1 API point.
+ try:
+ if self.repo.get_collaborator_permission(self.author) in ["admin", "write"]:
+ return
+ # There is a UnknownObjectException for this scenario, but this method
+ # does not use it.
+ except github.GithubException as e:
+ # 404 means the author was not found in the collaborator list, so we
+ # know they don't have push permissions. Anything else is a real API
+ # issue, raise it so it is visible.
+ if e.status != 404:
+ raise e
+
+ # 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} 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 +685,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 +746,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,
>From f7fb148a86c4301c5a69e8076d8c5f4516a8c678 Mon Sep 17 00:00:00 2001
From: David Spickett <david.spickett at linaro.org>
Date: Mon, 12 Feb 2024 09:30:20 +0000
Subject: [PATCH 2/4] _ -> -
---
.github/workflows/approved-prs.yml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/.github/workflows/approved-prs.yml b/.github/workflows/approved-prs.yml
index 6a037f7aead0e9..8902f968f53f18 100644
--- a/.github/workflows/approved-prs.yml
+++ b/.github/workflows/approved-prs.yml
@@ -9,7 +9,7 @@ on:
- submitted
jobs:
- merge_on_behalf_information_comment:
+ merge-on-behalf-information-comment:
runs-on: ubuntu-latest
permissions:
pull-requests: write
>From 582aac77a66f116b77d7427acf7bed9ef43948b2 Mon Sep 17 00:00:00 2001
From: David Spickett <david.spickett at linaro.org>
Date: Mon, 12 Feb 2024 15:43:07 +0000
Subject: [PATCH 3/4] Address reviewer instead.
---
.github/workflows/approved-prs.yml | 2 +-
llvm/utils/git/github-automation.py | 5 +----
2 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/.github/workflows/approved-prs.yml b/.github/workflows/approved-prs.yml
index 8902f968f53f18..baba8775b81188 100644
--- a/.github/workflows/approved-prs.yml
+++ b/.github/workflows/approved-prs.yml
@@ -35,4 +35,4 @@ jobs:
--token '${{ secrets.GITHUB_TOKEN }}' \
pr-merge-on-behalf-information \
--issue-number "${{ github.event.pull_request.number }}" \
- --author "${{ github.event.pull_request.user.login }}"
+ --author "${{ github.event.review.user.login }}"
diff --git a/llvm/utils/git/github-automation.py b/llvm/utils/git/github-automation.py
index c7483d4e6863d3..d775886d871045 100755
--- a/llvm/utils/git/github-automation.py
+++ b/llvm/utils/git/github-automation.py
@@ -328,10 +328,7 @@ def run(self) -> bool:
# This text is using Markdown formatting.
comment = f"""\
{self.COMMENT_TAG}
-@{self.author} 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.author} the PR author does not have permission to merge their own PRs yet. Please merge on their behalf."""
self.pr.as_issue().create_comment(comment)
return True
>From 1e566a4e0907e530b5fcaa5db43f41a84c1fbdb9 Mon Sep 17 00:00:00 2001
From: David Spickett <david.spickett at linaro.org>
Date: Mon, 12 Feb 2024 15:52:25 +0000
Subject: [PATCH 4/4] We need reviewer and author names.
---
.github/workflows/approved-prs.yml | 3 ++-
llvm/utils/git/github-automation.py | 12 +++++++++---
2 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/.github/workflows/approved-prs.yml b/.github/workflows/approved-prs.yml
index baba8775b81188..309a9217e42d31 100644
--- a/.github/workflows/approved-prs.yml
+++ b/.github/workflows/approved-prs.yml
@@ -35,4 +35,5 @@ jobs:
--token '${{ secrets.GITHUB_TOKEN }}' \
pr-merge-on-behalf-information \
--issue-number "${{ github.event.pull_request.number }}" \
- --author "${{ github.event.review.user.login }}"
+ --author "${{ github.event.pull_request.user.login }}" \
+ --reviewer "${{ github.event.review.user.login }}"
diff --git a/llvm/utils/git/github-automation.py b/llvm/utils/git/github-automation.py
index d775886d871045..cc7f57e34de930 100755
--- a/llvm/utils/git/github-automation.py
+++ b/llvm/utils/git/github-automation.py
@@ -301,10 +301,13 @@ def run(self) -> bool:
class PRMergeOnBehalfInformation:
COMMENT_TAG = "<!--LLVM MERGE ON BEHALF INFORMATION COMMENT-->\n"
- def __init__(self, token: str, repo: str, pr_number: int, author: str):
+ def __init__(
+ self, token: str, repo: str, pr_number: int, author: str, reviewer: str
+ ):
self.repo = github.Github(token).get_repo(repo)
self.pr = self.repo.get_issue(pr_number).as_pull_request()
self.author = author
+ self.reviewer = reviewer
def run(self) -> bool:
# Check this first because it only costs 1 API point.
@@ -328,7 +331,7 @@ def run(self) -> bool:
# This text is using Markdown formatting.
comment = f"""\
{self.COMMENT_TAG}
-@{self.author} the PR author does not have permission to merge their own PRs yet. Please merge on their behalf."""
+@{self.reviewer} the PR author does not have permission to merge their own PRs yet. Please merge on their behalf."""
self.pr.as_issue().create_comment(comment)
return True
@@ -689,6 +692,9 @@ def execute_command(self) -> bool:
"--issue-number", type=int, required=True
)
pr_merge_on_behalf_information_parser.add_argument("--author", type=str, required=True)
+pr_merge_on_behalf_information_parser.add_argument(
+ "--reviewer", type=str, required=True
+)
release_workflow_parser = subparsers.add_parser("release-workflow")
release_workflow_parser.add_argument(
@@ -745,7 +751,7 @@ def execute_command(self) -> bool:
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
+ args.token, args.repo, args.issue_number, args.author, args.reviewer
)
pr_merge_on_behalf_information.run()
elif args.command == "release-workflow":
More information about the llvm-commits
mailing list