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.

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

[#12194] Add simple workflow to vet pull requests #12196

Merged
merged 6 commits into from Mar 14, 2023
Merged

Conversation

zhaojj2209
Copy link
Contributor

@zhaojj2209 zhaojj2209 commented Mar 11, 2023

Fixes #12194
Part of #11383

Adds simple Github Actions workflow using actions/github-script to check PRs. The bot currently checks for the following:

  • Title: starts with issue number in square brackets, followed by a space
  • Description: contains either "Fixes" or "Part of", followed by the issue number. Only "Fixes" is checked since it is the keyword used in our PR template.

More checks can be added down the line as and when needed. The bot also only checks PRs that are newly opened/reopened, so as to avoid spam.

@github-actions
Copy link

Hi @zhaojj2209, thank you for your interest in contributing to TEAMMATES!
However, your PR does not appear to follow our contribution guidelines:

  • Title must start with the issue number the PR is fixing in square brackets, e.g. [#<issue-number>]
  • Description must reference the issue number the PR is fixing, e.g. Fixes #<issue-number> (or Part of #<issue-number> if the PR does not addres the issue fully)

Please address the above before we proceed to review your PR.

@zhaojj2209 zhaojj2209 changed the title Add simple workflow to vet pull requests [#12194] Add simple workflow to vet pull requests Mar 11, 2023
Copy link
Member

@samuelfangjw samuelfangjw left a comment

Choose a reason for hiding this comment

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

Seems like this doesn't check the title/pr after it is edited. Is this something we would want?

@zhaojj2209
Copy link
Contributor Author

Seems like this doesn't check the title/pr after it is edited. Is this something we would want?

Github Actions has an edited type for PRs which I believe is triggered on editing PR title/body, but I intentionally left it out. Back when the old bot was active, there was a case where a contributor opened a PR then proceeded to spam title/description edits, resulting in maintainers watching the repo being spammed with comment notifications. There is therefore a trade-off between either higher workload for the maintainers (if the action does not rerun on edit) or higher risk of spam (if the action does rerun on edit).

@damithc @wkurniawan07 Would appreciate your input on this!

@samuelfangjw
Copy link
Member

Github Actions has an edited type for PRs which I believe is triggered on editing PR title/body, but I intentionally left it out. Back when the old bot was active, there was a case where a contributor opened a PR then proceeded to spam title/description edits, resulting in maintainers watching the repo being spammed with comment notifications. There is therefore a trade-off between either higher workload for the maintainers (if the action does not rerun on edit) or higher risk of spam (if the action does rerun on edit).

Have similar thoughts, my opinion is it's ok to leave it as such. Maintainers can still manually verify/edit if there are issues.

.github/workflows/pr.yml Outdated Show resolved Hide resolved
@zhaojj2209 zhaojj2209 added the s.ToReview The PR is waiting for review(s) label Mar 11, 2023
@wkurniawan07
Copy link
Member

@zhaojj2209 I have no strong opinion on rerun on edit. As you have identified, either one has its own set of pros/cons.

Only "Fixes" is checked since it is the keyword used in our PR template.

The old bot checks for all 9 acceptable keywords, so it's better to retain the behaviour here.

.github/workflows/pr.yml Outdated Show resolved Hide resolved
@zhaojj2209 zhaojj2209 added s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging and removed s.ToReview The PR is waiting for review(s) labels Mar 14, 2023
@zhaojj2209 zhaojj2209 added this to the V8.25.0 milestone Mar 14, 2023
@zhaojj2209 zhaojj2209 merged commit 6e61b24 into master Mar 14, 2023
6 checks passed
@zhaojj2209 zhaojj2209 deleted the pr-workflow branch March 16, 2023 07:37
@zhaojj2209 zhaojj2209 restored the pr-workflow branch March 16, 2023 07:38
@zhaojj2209 zhaojj2209 deleted the pr-workflow branch March 16, 2023 08:00
@samuelfangjw samuelfangjw added the c.Feature User-facing feature; can be new feature or enhancement to existing feature label Mar 22, 2023
@samuelfangjw samuelfangjw added c.DevOps Process-related or build-related improvement and addition and removed c.Feature User-facing feature; can be new feature or enhancement to existing feature labels Mar 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c.DevOps Process-related or build-related improvement and addition s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add simple workflow to vet pull requests
4 participants