[llvm] [clang] [clang-tools-extra] [GitHub][workflows] Add buildbot information comment to first merged PR from a new contributor (PR #78292)
Nikita Popov via cfe-commits
cfe-commits at lists.llvm.org
Wed Jan 31 02:11:10 PST 2024
https://github.com/nikic updated https://github.com/llvm/llvm-project/pull/78292
>From 20822b4a2f8e228365c8fa912af18afc9956749e Mon Sep 17 00:00:00 2001
From: David Spickett <david.spickett at linaro.org>
Date: Tue, 16 Jan 2024 13:36:15 +0000
Subject: [PATCH 01/10] [GitHub][workflows] Add buildbot information comment to
first merged PR from a new contributor
This change adds a comment to the first PR from a new contributor that is merged,
which tells them what to expect post merge from the build bots.
How they will be notified, where to ask questions, that you're more likely
to be reverted than in other projects, etc.
To do this, I have added a hidden HTML comment to the new contributor greeting comment.
This workflow will look for that to tell if the author of the PR was
a new contributor at the time they opened the merge.
It has to be done this way because as soon as the PR is merged, they are
by GitHub's definition no longer a new contributor and I suspect that
their author assocication will be "contributor" instead.
I cannot 100% confirm that without a whole lot of effort and probably breaking
GitHub's terms of service, but it's fairly cheap to work around anyway.
It seems rare / almost impossible to reopen a PR in llvm at least, but
in case it does happen the buildbot info comment has its own hidden
HTML comment. If we find this we will not post another copy of the
same information.
An example PR can be found here: https://github.com/DavidSpickett/llvm-project/pull/84
(exact text content subject to change)
---
.github/workflows/merged-prs.yml | 36 +++++++++++++++
llvm/utils/git/github-automation.py | 71 +++++++++++++++++++++++++++++
2 files changed, 107 insertions(+)
create mode 100644 .github/workflows/merged-prs.yml
diff --git a/.github/workflows/merged-prs.yml b/.github/workflows/merged-prs.yml
new file mode 100644
index 0000000000000..1b1503610dac1
--- /dev/null
+++ b/.github/workflows/merged-prs.yml
@@ -0,0 +1,36 @@
+name: "Add buildbot information to first PRs from new contributors"
+
+permissions:
+ contents: read
+
+on:
+ # It's safe to use pull_request_target here, because we aren't checking out
+ # code from the pull request branch.
+ # See https://securitylab.github.com/research/github-actions-preventing-pwn-requests/
+ pull_request_target:
+ types:
+ - closed
+
+jobs:
+ buildbot_comment:
+ runs-on: ubuntu-latest
+ permissions:
+ pull-requests: write
+ if: >-
+ (github.repository == 'llvm/llvm-project') &&
+ (github.event.pull_request.merged == true)
+ steps:
+ - name: Setup Automation Script
+ run: |
+ curl -O -L --fail https://raw.githubusercontent.com/"$GITHUB_REPOSITORY"/main/llvm/utils/git/github-automation.py
+ curl -O -L --fail https://raw.githubusercontent.com/"$GITHUB_REPOSITORY"/main/llvm/utils/git/requirements.txt
+ chmod a+x github-automation.py
+ pip install -r requirements.txt
+
+ - name: Add Buildbot information comment
+ run: |
+ ./github-automation.py \
+ --token '${{ secrets.GITHUB_TOKEN }}' \
+ pr-buildbot-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 f78d91059ecd3..55659679536f4 100755
--- a/llvm/utils/git/github-automation.py
+++ b/llvm/utils/git/github-automation.py
@@ -208,6 +208,8 @@ def _get_curent_team(self) -> Optional[github.Team.Team]:
class PRGreeter:
+ COMMENT_TAG = "<!--LLVM NEW CONTRIBUTOR COMMENT-->\n"
+
def __init__(self, token: str, repo: str, pr_number: int):
repo = github.Github(token).get_repo(repo)
self.pr = repo.get_issue(pr_number).as_pull_request()
@@ -217,7 +219,9 @@ def run(self) -> bool:
# by a user new to LLVM and/or GitHub itself.
# This text is using Markdown formatting.
+
comment = f"""\
+{PRGreeter.COMMENT_TAG}
Thank you for submitting a Pull Request (PR) to the LLVM Project!
This PR will be automatically labeled and the relevant teams will be
@@ -240,6 +244,64 @@ def run(self) -> bool:
return True
+class PRBuildbotInformation:
+ COMMENT_TAG = "<!--LLVM BUILDBOT 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 should_comment(self) -> bool:
+ # As soon as a new contributor has a PR merged, they are no longer a new contributor.
+ # We can tell that they were a new contributor previously because we would have
+ # added a new contributor greeting comment when they opened the PR.
+ found_greeting = False
+ for comment in self.pr.as_issue().get_comments():
+ if PRGreeter.COMMENT_TAG in comment.body:
+ found_greeting = True
+ elif PRBuildbotInformation.COMMENT_TAG in comment.body:
+ # When an issue is reopened, then closed as merged again, we should not
+ # add a second comment. This event will be rare in practice as it seems
+ # like it's only possible when the main branch is still at the exact
+ # revision that the PR was merged on to, beyond that it's closed forever.
+ return False
+ return found_greeting
+
+ def run(self) -> bool:
+ if not self.should_comment():
+ return
+
+ # This text is using Markdown formatting.
+ comment = f"""\
+{PRBuildbotInformation.COMMENT_TAG}
+@{self.author} Congratulations on having your first Pull Request (PR) merged into the LLVM Project!
+
+Your changes will be combined with recent changes from other authors, then tested
+by our [build bots](https://lab.llvm.org/buildbot/#/console).
+
+If there is a problem with the build, all the change authors will receive an email
+describing the problem. Please check whether the problem has been caused by your
+change, as the change set may include many authors. If the problem affects many
+configurations, you may get many emails for the same problem.
+
+If you are using a GitHub `noreply` email address, you will not receive these emails.
+Instead, someone will comment on this PR to inform you of the issue.
+
+If you do not receive any reports of problems, no action is required from you.
+Your changes are working as expected, well done!
+
+If your change causes an ongoing issue, it may be reverted. This is a [normal part of LLVM development](https://llvm.org/docs/DeveloperPolicy.html#patch-reversion-policy)
+and is not a comment on yourself as an author.The revert commit (or a comment on this PR)
+should explain why it was reverted, how to fix your changes and merge them again.
+
+If you are unsure how to fix a problem, you can send questions in a reply
+to the notification email, add a comment to this PR, or 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
@@ -698,6 +760,10 @@ def execute_command(self) -> bool:
pr_greeter_parser = subparsers.add_parser("pr-greeter")
pr_greeter_parser.add_argument("--issue-number", type=int, required=True)
+pr_buildbot_information_parser = subparsers.add_parser("pr-buildbot-information")
+pr_buildbot_information_parser.add_argument("--issue-number", type=int, required=True)
+pr_buildbot_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",
@@ -751,6 +817,11 @@ def execute_command(self) -> bool:
elif args.command == "pr-greeter":
pr_greeter = PRGreeter(args.token, args.repo, args.issue_number)
pr_greeter.run()
+elif args.command == "pr-buildbot-information":
+ pr_buildbot_information = PRBuildbotInformation(
+ args.token, args.repo, args.issue_number, args.author
+ )
+ pr_buildbot_information.run()
elif args.command == "release-workflow":
release_workflow = ReleaseWorkflow(
args.token,
>From cffa5066450936bcdb6966db1f972088b1274fad Mon Sep 17 00:00:00 2001
From: David Spickett <david.spickett at linaro.org>
Date: Tue, 16 Jan 2024 15:29:55 +0000
Subject: [PATCH 02/10] use sparse checkout
---
.github/workflows/merged-prs.yml | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/.github/workflows/merged-prs.yml b/.github/workflows/merged-prs.yml
index 1b1503610dac1..44332c1c9fe0b 100644
--- a/.github/workflows/merged-prs.yml
+++ b/.github/workflows/merged-prs.yml
@@ -20,14 +20,19 @@ jobs:
(github.repository == 'llvm/llvm-project') &&
(github.event.pull_request.merged == true)
steps:
+ - name: Checkout Automation Script
+ uses: actions/checkout at v4
+ with:
+ sparse-checkout: llvm/utils/git/
+
- name: Setup Automation Script
+ working-directory: ./llvm/utils/git/
run: |
- curl -O -L --fail https://raw.githubusercontent.com/"$GITHUB_REPOSITORY"/main/llvm/utils/git/github-automation.py
- curl -O -L --fail https://raw.githubusercontent.com/"$GITHUB_REPOSITORY"/main/llvm/utils/git/requirements.txt
chmod a+x github-automation.py
pip install -r requirements.txt
- name: Add Buildbot information comment
+ working-directory: ./llvm/utils/git/
run: |
./github-automation.py \
--token '${{ secrets.GITHUB_TOKEN }}' \
>From dac42b9d02682fab7463a7324925bbf1d7f67a67 Mon Sep 17 00:00:00 2001
From: David Spickett <david.spickett at linaro.org>
Date: Tue, 16 Jan 2024 15:41:58 +0000
Subject: [PATCH 03/10] Missing space.
---
llvm/utils/git/github-automation.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/utils/git/github-automation.py b/llvm/utils/git/github-automation.py
index 55659679536f4..4ee843367b88e 100755
--- a/llvm/utils/git/github-automation.py
+++ b/llvm/utils/git/github-automation.py
@@ -292,7 +292,7 @@ def run(self) -> bool:
Your changes are working as expected, well done!
If your change causes an ongoing issue, it may be reverted. This is a [normal part of LLVM development](https://llvm.org/docs/DeveloperPolicy.html#patch-reversion-policy)
-and is not a comment on yourself as an author.The revert commit (or a comment on this PR)
+and is not a comment on yourself as an author. The revert commit (or a comment on this PR)
should explain why it was reverted, how to fix your changes and merge them again.
If you are unsure how to fix a problem, you can send questions in a reply
>From 97d302e37d96480d2e35d893d7485fadae13d91e Mon Sep 17 00:00:00 2001
From: David Spickett <david.spickett at linaro.org>
Date: Tue, 16 Jan 2024 15:49:15 +0000
Subject: [PATCH 04/10] Always checkout main not the PR itself.
---
.github/workflows/merged-prs.yml | 1 +
1 file changed, 1 insertion(+)
diff --git a/.github/workflows/merged-prs.yml b/.github/workflows/merged-prs.yml
index 44332c1c9fe0b..8773769376f20 100644
--- a/.github/workflows/merged-prs.yml
+++ b/.github/workflows/merged-prs.yml
@@ -24,6 +24,7 @@ jobs:
uses: actions/checkout at v4
with:
sparse-checkout: llvm/utils/git/
+ ref: main
- name: Setup Automation Script
working-directory: ./llvm/utils/git/
>From cda94ea2e1112613dd26745bb288be248b0b0f01 Mon Sep 17 00:00:00 2001
From: David Spickett <david.spickett at linaro.org>
Date: Wed, 17 Jan 2024 10:58:45 +0000
Subject: [PATCH 05/10] Run with python3 instead.
---
.github/workflows/merged-prs.yml | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/.github/workflows/merged-prs.yml b/.github/workflows/merged-prs.yml
index 8773769376f20..37fc6c67f000b 100644
--- a/.github/workflows/merged-prs.yml
+++ b/.github/workflows/merged-prs.yml
@@ -29,13 +29,12 @@ jobs:
- name: Setup Automation Script
working-directory: ./llvm/utils/git/
run: |
- chmod a+x github-automation.py
pip install -r requirements.txt
- name: Add Buildbot information comment
working-directory: ./llvm/utils/git/
run: |
- ./github-automation.py \
+ python3 ./github-automation.py \
--token '${{ secrets.GITHUB_TOKEN }}' \
pr-buildbot-information \
--issue-number "${{ github.event.pull_request.number }}" \
>From 07dd877bba5f78f2801ce75b2669b4db2e353d68 Mon Sep 17 00:00:00 2001
From: David Spickett <david.spickett at linaro.org>
Date: Wed, 17 Jan 2024 11:16:05 +0000
Subject: [PATCH 06/10] * suggest console view to track builds * new PR needed
for reland
---
llvm/utils/git/github-automation.py | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/llvm/utils/git/github-automation.py b/llvm/utils/git/github-automation.py
index 4ee843367b88e..5ee1a7976a9bf 100755
--- a/llvm/utils/git/github-automation.py
+++ b/llvm/utils/git/github-automation.py
@@ -278,22 +278,26 @@ def run(self) -> bool:
@{self.author} Congratulations on having your first Pull Request (PR) merged into the LLVM Project!
Your changes will be combined with recent changes from other authors, then tested
-by our [build bots](https://lab.llvm.org/buildbot/#/console).
+by our [build bots](https://lab.llvm.org/buildbot/).
-If there is a problem with the build, all the change authors will receive an email
+If there is a problem with a build, all the change authors will receive an email
describing the problem. Please check whether the problem has been caused by your
-change, as the change set may include many authors. If the problem affects many
+change specifically, as the change set may include many authors. If the problem affects many
configurations, you may get many emails for the same problem.
If you are using a GitHub `noreply` email address, you will not receive these emails.
Instead, someone will comment on this PR to inform you of the issue.
+You can also track the progress of your change using the [build bot console view](https://lab.llvm.org/buildbot/#/console).
+However, this page only shows recent changes. If yours is not listed there, rely on the notifications.
+
If you do not receive any reports of problems, no action is required from you.
Your changes are working as expected, well done!
If your change causes an ongoing issue, it may be reverted. This is a [normal part of LLVM development](https://llvm.org/docs/DeveloperPolicy.html#patch-reversion-policy)
and is not a comment on yourself as an author. The revert commit (or a comment on this PR)
-should explain why it was reverted, how to fix your changes and merge them again.
+should explain why it was reverted and how to fix your changes. Please open a new
+PR with the fixed changes and describe what was done to fix them.
If you are unsure how to fix a problem, you can send questions in a reply
to the notification email, add a comment to this PR, or ask on [Discord](https://discord.com/invite/xS7Z362).
>From 9cf9c0f855858ab79153d281386da196c2bff99c Mon Sep 17 00:00:00 2001
From: David Spickett <david.spickett at linaro.org>
Date: Tue, 23 Jan 2024 09:37:20 +0000
Subject: [PATCH 07/10] * "may" get emails, so we don't have to list all the
possible ways this may not happen. * Remove replying to emails because
again, there's some situations where that won't be helpful.
---
llvm/utils/git/github-automation.py | 24 +++++++++++++-----------
1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/llvm/utils/git/github-automation.py b/llvm/utils/git/github-automation.py
index 5ee1a7976a9bf..936b035192053 100755
--- a/llvm/utils/git/github-automation.py
+++ b/llvm/utils/git/github-automation.py
@@ -280,27 +280,29 @@ def run(self) -> bool:
Your changes will be combined with recent changes from other authors, then tested
by our [build bots](https://lab.llvm.org/buildbot/).
-If there is a problem with a build, all the change authors will receive an email
-describing the problem. Please check whether the problem has been caused by your
-change specifically, as the change set may include many authors. If the problem affects many
-configurations, you may get many emails for the same problem.
+If there is a problem with a build, you may recieve an email describing the problem.
+Please check whether the problem has been caused by your change specifically, as
+the build may include changes from many authors. If the same problem affects many builds,
+you may get many emails describing the same problem.
-If you are using a GitHub `noreply` email address, you will not receive these emails.
-Instead, someone will comment on this PR to inform you of the issue.
+You may also be notified of a problem by a comment added to this PR.
You can also track the progress of your change using the [build bot console view](https://lab.llvm.org/buildbot/#/console).
-However, this page only shows recent changes. If yours is not listed there, rely on the notifications.
+However this page only shows recent changes. If yours is not listed there, rely on
+the other notifications.
-If you do not receive any reports of problems, no action is required from you.
-Your changes are working as expected, well done!
+If you are unsure whether your change has caused a problem, you can ask questions in
+a comment on this PR or on [Discord](https://discord.com/invite/xS7Z362).
If your change causes an ongoing issue, it may be reverted. This is a [normal part of LLVM development](https://llvm.org/docs/DeveloperPolicy.html#patch-reversion-policy)
and is not a comment on yourself as an author. The revert commit (or a comment on this PR)
should explain why it was reverted and how to fix your changes. Please open a new
PR with the fixed changes and describe what was done to fix them.
-If you are unsure how to fix a problem, you can send questions in a reply
-to the notification email, add a comment to this PR, or ask on [Discord](https://discord.com/invite/xS7Z362).
+If you want to revert your own change, you can [create a new PR](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/incorporating-changes-from-a-pull-request/reverting-a-pull-request) to do that.
+
+If you do not receive any reports of problems, no action is required from you.
+Your changes are working as expected, well done!
"""
self.pr.as_issue().create_comment(comment)
return True
>From 7f9f76fcf7f45b2217b38cb0beaa0c80f7480981 Mon Sep 17 00:00:00 2001
From: David Spickett <david.spickett at linaro.org>
Date: Tue, 23 Jan 2024 09:44:29 +0000
Subject: [PATCH 08/10] Shuffling things around.
---
llvm/utils/git/github-automation.py | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/llvm/utils/git/github-automation.py b/llvm/utils/git/github-automation.py
index 936b035192053..3d2a051714487 100755
--- a/llvm/utils/git/github-automation.py
+++ b/llvm/utils/git/github-automation.py
@@ -278,28 +278,27 @@ def run(self) -> bool:
@{self.author} Congratulations on having your first Pull Request (PR) merged into the LLVM Project!
Your changes will be combined with recent changes from other authors, then tested
-by our [build bots](https://lab.llvm.org/buildbot/).
-
-If there is a problem with a build, you may recieve an email describing the problem.
-Please check whether the problem has been caused by your change specifically, as
-the build may include changes from many authors. If the same problem affects many builds,
-you may get many emails describing the same problem.
-
-You may also be notified of a problem by a comment added to this PR.
+by our [build bots](https://lab.llvm.org/buildbot/). If there is a problem with a build,
+you may recieve a report via email or a comment on this PR.
You can also track the progress of your change using the [build bot console view](https://lab.llvm.org/buildbot/#/console).
However this page only shows recent changes. If yours is not listed there, rely on
the other notifications.
+Please check whether problems have been caused by your change specifically, as
+the builds can include changes from many authors. It is not uncommon for your
+change to be included in a build that fails due to someone else's changes, or
+infrastructure issues.
+
If you are unsure whether your change has caused a problem, you can ask questions in
a comment on this PR or on [Discord](https://discord.com/invite/xS7Z362).
-If your change causes an ongoing issue, it may be reverted. This is a [normal part of LLVM development](https://llvm.org/docs/DeveloperPolicy.html#patch-reversion-policy)
+If your change does cause a problem, it may be reverted. This is a [normal part of LLVM development](https://llvm.org/docs/DeveloperPolicy.html#patch-reversion-policy)
and is not a comment on yourself as an author. The revert commit (or a comment on this PR)
should explain why it was reverted and how to fix your changes. Please open a new
PR with the fixed changes and describe what was done to fix them.
-If you want to revert your own change, you can [create a new PR](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/incorporating-changes-from-a-pull-request/reverting-a-pull-request) to do that.
+If you want to revert your own change, you can [create a revert PR](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/incorporating-changes-from-a-pull-request/reverting-a-pull-request) to do that.
If you do not receive any reports of problems, no action is required from you.
Your changes are working as expected, well done!
>From a6ae12532b9a1a3c9b0af0e77a1f92cac8ebf6ad Mon Sep 17 00:00:00 2001
From: David Spickett <david.spickett at linaro.org>
Date: Tue, 30 Jan 2024 14:17:20 +0000
Subject: [PATCH 09/10] Link to the new info section
---
llvm/utils/git/github-automation.py | 20 +++++---------------
1 file changed, 5 insertions(+), 15 deletions(-)
diff --git a/llvm/utils/git/github-automation.py b/llvm/utils/git/github-automation.py
index 3d2a051714487..3b0eb56981065 100755
--- a/llvm/utils/git/github-automation.py
+++ b/llvm/utils/git/github-automation.py
@@ -279,29 +279,19 @@ def run(self) -> bool:
Your changes will be combined with recent changes from other authors, then tested
by our [build bots](https://lab.llvm.org/buildbot/). If there is a problem with a build,
-you may recieve a report via email or a comment on this PR.
-
-You can also track the progress of your change using the [build bot console view](https://lab.llvm.org/buildbot/#/console).
-However this page only shows recent changes. If yours is not listed there, rely on
-the other notifications.
+you may recieve a report in an email or a comment on this PR.
Please check whether problems have been caused by your change specifically, as
the builds can include changes from many authors. It is not uncommon for your
change to be included in a build that fails due to someone else's changes, or
infrastructure issues.
-If you are unsure whether your change has caused a problem, you can ask questions in
-a comment on this PR or on [Discord](https://discord.com/invite/xS7Z362).
-
-If your change does cause a problem, it may be reverted. This is a [normal part of LLVM development](https://llvm.org/docs/DeveloperPolicy.html#patch-reversion-policy)
-and is not a comment on yourself as an author. The revert commit (or a comment on this PR)
-should explain why it was reverted and how to fix your changes. Please open a new
-PR with the fixed changes and describe what was done to fix them.
+How to do this, and the rest of the post-merge process, is covered in detail [here](https://llvm.org/docs/MyFirstTypoFix.html#myfirsttypofix-issues-after-landing-your-pr).
-If you want to revert your own change, you can [create a revert PR](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/incorporating-changes-from-a-pull-request/reverting-a-pull-request) to do that.
+If your change does cause a problem, it may be reverted, or you can revert it yourself.
+This is a normal part of [LLVM development](https://llvm.org/docs/DeveloperPolicy.html#patch-reversion-policy). You can fix your changes and open a new PR to merge them again.
-If you do not receive any reports of problems, no action is required from you.
-Your changes are working as expected, well done!
+If you don't get any reports, no action is required from you. Your changes are working as expected, well done!
"""
self.pr.as_issue().create_comment(comment)
return True
>From d9c09b0b764f652ca00d6691f69264a49a2a2954 Mon Sep 17 00:00:00 2001
From: David Spickett <david.spickett at linaro.org>
Date: Tue, 30 Jan 2024 15:44:51 +0000
Subject: [PATCH 10/10] note strange line breaking
---
llvm/utils/git/github-automation.py | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/llvm/utils/git/github-automation.py b/llvm/utils/git/github-automation.py
index 3b0eb56981065..2f315cb04eee7 100755
--- a/llvm/utils/git/github-automation.py
+++ b/llvm/utils/git/github-automation.py
@@ -272,7 +272,9 @@ def run(self) -> bool:
if not self.should_comment():
return
- # This text is using Markdown formatting.
+ # This text is using Markdown formatting. Some of the lines are longer
+ # than others so that the final text is some reasonable looking paragraphs
+ # after the long URLs are rendered.
comment = f"""\
{PRBuildbotInformation.COMMENT_TAG}
@{self.author} Congratulations on having your first Pull Request (PR) merged into the LLVM Project!
More information about the cfe-commits
mailing list