Skip to content

Commit 44ba4c7

Browse files
[GitHub][workflows] Add buildbot information comment to first merged PR from a new contributor (#78292)
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. The information overlaps with, and links to https://llvm.org/docs/MyFirstTypoFix.html#myfirsttypofix-issues-after-landing-your-pr. So that users who simply read the email are still aware, and know where to follow up if they do get reports. 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 association 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.
1 parent db1fbd6 commit 44ba4c7

File tree

2 files changed

+109
-0
lines changed

2 files changed

+109
-0
lines changed

.github/workflows/merged-prs.yml

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
name: "Add buildbot information to first PRs from new contributors"
2+
3+
permissions:
4+
contents: read
5+
6+
on:
7+
# It's safe to use pull_request_target here, because we aren't checking out
8+
# code from the pull request branch.
9+
# See https://securitylab.github.com/research/github-actions-preventing-pwn-requests/
10+
pull_request_target:
11+
types:
12+
- closed
13+
14+
jobs:
15+
buildbot_comment:
16+
runs-on: ubuntu-latest
17+
permissions:
18+
pull-requests: write
19+
if: >-
20+
(github.repository == 'llvm/llvm-project') &&
21+
(github.event.pull_request.merged == true)
22+
steps:
23+
- name: Checkout Automation Script
24+
uses: actions/checkout@v4
25+
with:
26+
sparse-checkout: llvm/utils/git/
27+
ref: main
28+
29+
- name: Setup Automation Script
30+
working-directory: ./llvm/utils/git/
31+
run: |
32+
pip install -r requirements.txt
33+
34+
- name: Add Buildbot information comment
35+
working-directory: ./llvm/utils/git/
36+
run: |
37+
python3 ./github-automation.py \
38+
--token '${{ secrets.GITHUB_TOKEN }}' \
39+
pr-buildbot-information \
40+
--issue-number "${{ github.event.pull_request.number }}" \
41+
--author "${{ github.event.pull_request.user.login }}"

llvm/utils/git/github-automation.py

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,8 @@ def _get_curent_team(self) -> Optional[github.Team.Team]:
208208

209209

210210
class PRGreeter:
211+
COMMENT_TAG = "<!--LLVM NEW CONTRIBUTOR COMMENT-->\n"
212+
211213
def __init__(self, token: str, repo: str, pr_number: int):
212214
repo = github.Github(token).get_repo(repo)
213215
self.pr = repo.get_issue(pr_number).as_pull_request()
@@ -217,7 +219,9 @@ def run(self) -> bool:
217219
# by a user new to LLVM and/or GitHub itself.
218220

219221
# This text is using Markdown formatting.
222+
220223
comment = f"""\
224+
{PRGreeter.COMMENT_TAG}
221225
Thank you for submitting a Pull Request (PR) to the LLVM Project!
222226
223227
This PR will be automatically labeled and the relevant teams will be
@@ -240,6 +244,61 @@ def run(self) -> bool:
240244
return True
241245

242246

247+
class PRBuildbotInformation:
248+
COMMENT_TAG = "<!--LLVM BUILDBOT INFORMATION COMMENT-->\n"
249+
250+
def __init__(self, token: str, repo: str, pr_number: int, author: str):
251+
repo = github.Github(token).get_repo(repo)
252+
self.pr = repo.get_issue(pr_number).as_pull_request()
253+
self.author = author
254+
255+
def should_comment(self) -> bool:
256+
# As soon as a new contributor has a PR merged, they are no longer a new contributor.
257+
# We can tell that they were a new contributor previously because we would have
258+
# added a new contributor greeting comment when they opened the PR.
259+
found_greeting = False
260+
for comment in self.pr.as_issue().get_comments():
261+
if PRGreeter.COMMENT_TAG in comment.body:
262+
found_greeting = True
263+
elif PRBuildbotInformation.COMMENT_TAG in comment.body:
264+
# When an issue is reopened, then closed as merged again, we should not
265+
# add a second comment. This event will be rare in practice as it seems
266+
# like it's only possible when the main branch is still at the exact
267+
# revision that the PR was merged on to, beyond that it's closed forever.
268+
return False
269+
return found_greeting
270+
271+
def run(self) -> bool:
272+
if not self.should_comment():
273+
return
274+
275+
# This text is using Markdown formatting. Some of the lines are longer
276+
# than others so that the final text is some reasonable looking paragraphs
277+
# after the long URLs are rendered.
278+
comment = f"""\
279+
{PRBuildbotInformation.COMMENT_TAG}
280+
@{self.author} Congratulations on having your first Pull Request (PR) merged into the LLVM Project!
281+
282+
Your changes will be combined with recent changes from other authors, then tested
283+
by our [build bots](https://lab.llvm.org/buildbot/). If there is a problem with a build,
284+
you may recieve a report in an email or a comment on this PR.
285+
286+
Please check whether problems have been caused by your change specifically, as
287+
the builds can include changes from many authors. It is not uncommon for your
288+
change to be included in a build that fails due to someone else's changes, or
289+
infrastructure issues.
290+
291+
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).
292+
293+
If your change does cause a problem, it may be reverted, or you can revert it yourself.
294+
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.
295+
296+
If you don't get any reports, no action is required from you. Your changes are working as expected, well done!
297+
"""
298+
self.pr.as_issue().create_comment(comment)
299+
return True
300+
301+
243302
def setup_llvmbot_git(git_dir="."):
244303
"""
245304
Configure the git repo in `git_dir` with the llvmbot account so
@@ -585,6 +644,10 @@ def execute_command(self) -> bool:
585644
pr_greeter_parser = subparsers.add_parser("pr-greeter")
586645
pr_greeter_parser.add_argument("--issue-number", type=int, required=True)
587646

647+
pr_buildbot_information_parser = subparsers.add_parser("pr-buildbot-information")
648+
pr_buildbot_information_parser.add_argument("--issue-number", type=int, required=True)
649+
pr_buildbot_information_parser.add_argument("--author", type=str, required=True)
650+
588651
release_workflow_parser = subparsers.add_parser("release-workflow")
589652
release_workflow_parser.add_argument(
590653
"--llvm-project-dir",
@@ -633,6 +696,11 @@ def execute_command(self) -> bool:
633696
elif args.command == "pr-greeter":
634697
pr_greeter = PRGreeter(args.token, args.repo, args.issue_number)
635698
pr_greeter.run()
699+
elif args.command == "pr-buildbot-information":
700+
pr_buildbot_information = PRBuildbotInformation(
701+
args.token, args.repo, args.issue_number, args.author
702+
)
703+
pr_buildbot_information.run()
636704
elif args.command == "release-workflow":
637705
release_workflow = ReleaseWorkflow(
638706
args.token,

0 commit comments

Comments
 (0)