Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[GitHub] Add Greeting comment to new contributor's PRs with useful information #72249

Closed
wants to merge 1 commit into from
Closed

[GitHub] Add Greeting comment to new contributor's PRs with useful information #72249

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions .github/workflows/pr-greeter.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
name: PR Greeter

on:
pull_request_target:
types: [ opened ]

permissions:
content: read

jobs:
greet:
runs-on: ubuntu-latest
permissions:
pull-requests: write
if: github.repository == 'llvm/llvm-project' && (github.event.pull_request.author_association == 'FIRST_TIME_CONTRIBUTOR' || github.event.pull_request.author_association == 'FIRST_TIMER')
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: Greet Author
run: |
./github-automation.py \
--token '${{ secrets.GITHUB_TOKEN }}' \
pr-greeter \
--issue-number "${{ github.event.pull_request.number }}"
36 changes: 36 additions & 0 deletions llvm/utils/git/github-automation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,36 @@ def _get_curent_team(self) -> Optional[github.Team.Team]:
return None


class PRGreeter:
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()

def run(self) -> bool:
# We assume that this is only called for a PR that has just been opened
# by a user new to LLVM and/or GitHub itself.

# This text is using Markdown formatting.
comment = f"""\
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this comment content. The question is do we want to add this to every pull request or do we want to only use it on certain pull requests, like from new contributors.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds like folks would like it limited, and starting with only new contributors can't do any harm anyway, so I'll implement that.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment content in proposed in the PR right now seems geared towards new (or the very occasional) contributor. Therefore, it would be best to only put this on PRs created by those type of contributors.
I guess that one heuristic to cover "very occasional" contributor could be to put this on every PR where the contributor hasn't contributed to LLVM in e.g. a year? Or the first PR for that contributor this year (which would result in frequent contributors seeing this once per year)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have seen some prebuilt actions that list all the PRs for a project and keep searching backwards until they see (or don't see) a PR from the same author. Though I wonder how that would work on LLVM's scale.

For now I've used a filter in the job YAML which is more brittle but will catch a decent number of cases.

Thank you for submitting a Pull Request (PR) to the LLVM Project!

You can add reviewers by using the "Reviewers" section on this page.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about a sentence before this that explains that prs are automatically labeled and the respective teams for these labels will automatically be pinged. I would still keep the rest of the reviewer part, because the user might want to ping specific people for their involvement in that part of the code-base.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or might this be obvious by the time the action ran, because the labeling and team ping already happened?

Copy link
Collaborator Author

@DavidSpickett DavidSpickett Nov 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A note is a good idea regardless, but as it stands, there's no explicit ordering of the jobs. This could finish before or after the labeller.

Maybe it would be better to include this workflow as the first job as the labeler (.github/workflows/new-prs.yml) and have the labeler depend on that job having run (or been skipped). Then we would always have greeting (or not) - labelling - subscribers.

And in theory, only the author gets the email with the greeting notification.


If this is not working for you, it's probably because you don't have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using `@` followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the [LLVM GitHub User Guide](https://llvm.org/docs/GitHub.html).

You can also ask questions in a comment on this PR, on the [LLVM Discord](https://discord.com/invite/xS7Z362) or on the [forums](https://discourse.llvm.org/)."""
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
Expand Down Expand Up @@ -655,6 +685,9 @@ def execute_command(self) -> bool:
pr_subscriber_parser.add_argument("--label-name", type=str, required=True)
pr_subscriber_parser.add_argument("--issue-number", type=int, required=True)

pr_greeter_parser = subparsers.add_parser("pr-greeter")
pr_greeter_parser.add_argument("--issue-number", type=int, required=True)

release_workflow_parser = subparsers.add_parser("release-workflow")
release_workflow_parser.add_argument(
"--llvm-project-dir",
Expand Down Expand Up @@ -705,6 +738,9 @@ def execute_command(self) -> bool:
args.token, args.repo, args.issue_number, args.label_name
)
pr_subscriber.run()
elif args.command == "pr-greeter":
pr_greeter = PRGreeter(args.token, args.repo, args.issue_number)
pr_greeter.run()
elif args.command == "release-workflow":
release_workflow = ReleaseWorkflow(
args.token,
Expand Down
Loading