[llvm] Revert "[GitHub][workflows] Ask reviewers to merge PRs when author cannot" (PR #81722)

via llvm-commits llvm-commits at lists.llvm.org
Wed Feb 14 02:32:08 PST 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-github-workflow

Author: David Spickett (DavidSpickett)

<details>
<summary>Changes</summary>

Reverts llvm/llvm-project#<!-- -->81142

Turns out that when using the `pull_request_review` event, you cannot have write permissions for `GITHUB_TOKEN`. So this workflow is always going to fail with:
```
github.GithubException.GithubException: 403 {"message": "Resource not accessible by integration", "documentation_url": "https://docs.github.com/rest/issues/comments#create-an-issue-comment"}
```

https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request_review

See also:
https://github.com/orgs/community/discussions/26651
https://github.com/orgs/community/discussions/55940

So we will have to find another trigger for this, that is able to add comments.

I think this worked while testing on my fork because the PR was from one branch of my fork to another of that same fork.

---
Full diff: https://github.com/llvm/llvm-project/pull/81722.diff


2 Files Affected:

- (removed) .github/workflows/approved-prs.yml (-39) 
- (modified) llvm/utils/git/github-automation.py (-65) 


``````````diff
diff --git a/.github/workflows/approved-prs.yml b/.github/workflows/approved-prs.yml
deleted file mode 100644
index 309a9217e42d31..00000000000000
--- a/.github/workflows/approved-prs.yml
+++ /dev/null
@@ -1,39 +0,0 @@
-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 }}" \
-            --reviewer "${{ github.event.review.user.login }}"
diff --git a/llvm/utils/git/github-automation.py b/llvm/utils/git/github-automation.py
index ccef274c4c1f7c..b475eff06fc3eb 100755
--- a/llvm/utils/git/github-automation.py
+++ b/llvm/utils/git/github-automation.py
@@ -298,55 +298,6 @@ 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, 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 can_merge(self, user: str) -> bool:
-        try:
-            return self.repo.get_collaborator_permission(user) in ["admin", "write"]
-        # 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
-            return False
-
-    def run(self) -> bool:
-        # Check this first because it only costs 1 API point.
-        if self.can_merge(self.author):
-            return
-
-        # 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.
-        if self.can_merge(self.reviewer):
-            comment = f"""\
-{self.COMMENT_TAG}
-@{self.reviewer} the PR author does not have permission to merge their own PRs yet. Please merge on their behalf."""
-        else:
-            comment = f"""\
-{self.COMMENT_TAG}
-@{self.reviewer} the author of this PR does not have permission to merge and neither do you.
-Please find someone who has merge permissions who can merge it on the author's behalf. This could be one of the other reviewers or you can ask on [Discord](https://discord.com/invite/xS7Z362)."""
-
-        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
@@ -714,17 +665,6 @@ 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)
-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(
     "--llvm-project-dir",
@@ -784,11 +724,6 @@ 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, args.reviewer
-    )
-    pr_merge_on_behalf_information.run()
 elif args.command == "release-workflow":
     release_workflow = ReleaseWorkflow(
         args.token,

``````````

</details>


https://github.com/llvm/llvm-project/pull/81722


More information about the llvm-commits mailing list