llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
29.12k stars 12.01k forks source link

[infrastructure] github-automation: pings missing in backport PRs #109429

Open kleisauke opened 1 month ago

kleisauke commented 1 month ago

Looking at the GitHub automation routines script (i.e. the one that handles the /cherry-pick command), I think the last line of this code snippet is wrongly indented: https://github.com/llvm/llvm-project/blob/c320df4a2c9d7be10caea9a423d2bfbdcaae6a39/llvm/utils/git/github-automation.py#L497-L500

i.e. that line should probably be part of the for loop:

--- a/llvm/utils/git/github-automation.py
+++ b/llvm/utils/git/github-automation.py
@@ -497,7 +497,7 @@ class ReleaseWorkflow:
                 for review in pull.get_reviews():
                     if review.state != "APPROVED":
                         continue
-                reviewers.append(review.user.login)
+                    reviewers.append(review.user.login)
         if len(reviewers):
             message = "{} What do you think about merging this PR to the release branch?".format(
                 " ".join(["@" + r for r in reviewers])

Noticed this while using this script in another project (see https://github.com/libvips/libvips/pull/4162):

error: Failed while searching for reviewers local variable 'review' referenced before assignment

https://github.com/kleisauke/libvips/actions/runs/10957466259/job/30425668132#step:4:19

llvmbot commented 1 month ago

@llvm/issue-subscribers-infrastructure

Author: Kleis Auke Wolthuizen (kleisauke)

<!--IGNORE--> Looking at the GitHub automation routines script (i.e. the one that handles the `/cherry-pick` command), I think the last line of this code snippet is wrongly indented: https://github.com/llvm/llvm-project/blob/c320df4a2c9d7be10caea9a423d2bfbdcaae6a39/llvm/utils/git/github-automation.py#L497-L500 i.e. that line should probably be part of the for loop: ```diff --- a/llvm/utils/git/github-automation.py +++ b/llvm/utils/git/github-automation.py @@ -497,7 +497,7 @@ class ReleaseWorkflow: for review in pull.get_reviews(): if review.state != "APPROVED": continue - reviewers.append(review.user.login) + reviewers.append(review.user.login) if len(reviewers): message = "{} What do you think about merging this PR to the release branch?".format( " ".join(["@" + r for r in reviewers]) ``` Noticed this while using this script in another project (see https://github.com/libvips/libvips/pull/4162): ``` error: Failed while searching for reviewers local variable 'review' referenced before assignment ``` https://github.com/kleisauke/libvips/actions/runs/10957466259/job/30425668132#step:4:19
llvmbot commented 1 month ago

Looking at the GitHub automation routines script (i.e. the one that handles the /cherry-pick command), I think the last line of this code snippet is wrongly indented: https://github.com/llvm/llvm-project/blob/c320df4a2c9d7be10caea9a423d2bfbdcaae6a39/llvm/utils/git/github-automation.py#L497-L500

i.e. that line should probably be part of the for loop:

--- a/llvm/utils/git/github-automation.py
+++ b/llvm/utils/git/github-automation.py
@@ -497,7 +497,7 @@ class ReleaseWorkflow:
                for review in pull.get_reviews():
                    if review.state != "APPROVED":
                        continue
-                reviewers.append(review.user.login)
+                    reviewers.append(review.user.login)
        if len(reviewers):
            message = "{} What do you think about merging this PR to the release branch?".format(
                " ".join(["@" + r for r in reviewers])

Noticed this while using this script in another project (see https://github.com/libvips/libvips/pull/4162):

error: Failed while searching for reviewers local variable 'review' referenced before assignment

https://github.com/kleisauke/libvips/actions/runs/10957466259/job/30425668132#step:4:19

Error: Command failed due to missing milestone.

llvmbot commented 1 month ago

@llvm/issue-subscribers-infrastructure

Author: Kleis Auke Wolthuizen (kleisauke)

<!--IGNORE--> Looking at the GitHub automation routines script (i.e. the one that handles the `/cherry-pick` command), I think the last line of this code snippet is wrongly indented: https://github.com/llvm/llvm-project/blob/c320df4a2c9d7be10caea9a423d2bfbdcaae6a39/llvm/utils/git/github-automation.py#L497-L500 i.e. that line should probably be part of the for loop: ```diff --- a/llvm/utils/git/github-automation.py +++ b/llvm/utils/git/github-automation.py @@ -497,7 +497,7 @@ class ReleaseWorkflow: for review in pull.get_reviews(): if review.state != "APPROVED": continue - reviewers.append(review.user.login) + reviewers.append(review.user.login) if len(reviewers): message = "{} What do you think about merging this PR to the release branch?".format( " ".join(["@" + r for r in reviewers]) ``` Noticed this while using this script in another project (see https://github.com/libvips/libvips/pull/4162): ``` error: Failed while searching for reviewers local variable 'review' referenced before assignment ``` https://github.com/kleisauke/libvips/actions/runs/10957466259/job/30425668132#step:4:19

Error: Command failed due to missing milestone.

kleisauke commented 1 week ago

Triage: This issue is likely the root cause of sometimes missing pings, as noted in https://github.com/llvm/llvm-project/pull/107146#issuecomment-2327419871 and https://github.com/llvm/llvm-project/pull/112258#issuecomment-2412339071. /cc @mstorsjo FYI.

The "error" only occurs when a PR is merged without approval, which likely doesn't apply to LLVM. It's also properly caught here anyway: https://github.com/llvm/llvm-project/blob/da9fece01278b85a3ead8f6a72faf7762778f44a/llvm/utils/git/github-automation.py#L590-L593

I've clarified the issue title accordingly.