Skip to content

Conversation

@damccorm
Copy link
Contributor

Right now, we don't have a well defined automated system for staying on top of pull request reviews - we rely on contributors being able to find the correct OWNERS file and committers manually triaging/calling attention to old pull requests. This is the continuation of an effort to improve our automation around this. Design doc for the full effort here Part 1 was #16898

I have the code to handle new prs and pr updates in a repo here - https://github.com/damccorm/beam-pr-bot-demo - this is a subset of the damccorm/beam-pr-bot-demo#1 split up to try to keep reviews manageable. This PR introduces the files used to process new prs and pr updates and it builds on the shared files introduced in the last PR. This contains most of the business logic of the tool. It also contains a .gitignore and Commands.md file I missed in the last PR.

Initially, I am filtering all bot activity prs with the Go label. This is to minimize the blast radius of the automation in case it doesn't work perfectly out the gate or we find it unhelpful. Eventually, I plan on expanding to the rest of the repo.

You can see see the completed portions of the bot in action in damccorm/beam-pr-bot-demo#3 and damccorm/beam-pr-bot-demo#4 (they show different use cases and exercise almost all of the behavior of the bot).


Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

  • Choose reviewer(s) and mention them in a comment (R: @username).
  • Format the pull request title like [BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replace BEAM-XXX with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
  • Update CHANGES.md with noteworthy changes.
  • If this contribution is large, please file an Apache Individual Contributor License Agreement.

See the Contributor Guide for more tips on how to make review process smoother.

To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md

GitHub Actions Tests Status (on master branch)

Build python source distribution and wheels
Python tests
Java tests

See CI.md for more information about GitHub Actions CI.

@damccorm
Copy link
Contributor Author

R: @KonradJanica

Copy link
Contributor

@lostluck lostluck left a comment

Choose a reason for hiding this comment

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

Only one question from me. I don't know enough TS to comment on the specifics though.

// 4) Are closed
// 5) Have already been processed
// 6) Have notifications stopped
// 7) The pr doesn't contain the go label (temporary). TODO(damccorm) - remove this when we're ready to roll this out to everyone.
Copy link
Contributor

Choose a reason for hiding this comment

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

Question: After this Go only phase, what happens when a PR has multiple labels? Will it try to assign someone from each label, or will it do some sort of consolidation or limit to # reviewers assigned?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It will assign a reviewer from each configured label (though it should only look for one committer)

@damccorm
Copy link
Contributor Author

R: @lostluck for final review

Copy link
Contributor

@lostluck lostluck left a comment

Choose a reason for hiding this comment

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

LGTM. Merging it in!

@lostluck lostluck merged commit 3070d5f into apache:master Mar 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants