-
Notifications
You must be signed in to change notification settings - Fork 4.5k
Expand pr bot to python #21791
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
Expand pr bot to python #21791
Conversation
|
Can one of the admins verify this patch? |
3 similar comments
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
…corm/beam into users/damccorm/prbot-python
|
R: @tvalentyn |
| label.name.toLowerCase() === "python" | ||
| ) | ||
| ) { | ||
| console.log("Does not contain the go or python labels - skipping"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for my education, where can these logs be inspected?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The actions console for the run - https://github.com/apache/beam/actions/workflows/pr-bot-new-prs.yml
| */ | ||
| function needsProcessed(pull: any, prState: typeof Pr): boolean { | ||
| if (!pull.labels.find((label) => label.name.toLowerCase() === "go")) { | ||
| if ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
re item 6) above : for my education, what does it mean to have notifications stopped?
item 7 is out of sync now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Anytime a reviewer manually requests review (e.g. R: @user), the bot turns itself off (and comments that its doing so). For example - #21852 (comment)
Fixed the comment
| if (createdAt < firstPrToProcess) { | ||
| if ( | ||
| createdAt < firstPrToProcess && | ||
| !pull.labels.find((label) => label.name.toLowerCase() === "go") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need to exclude go here given that the bot has already been enabled for go? and probably all outstanding go prs have been processed already, except for maybe a couple go of PRs that happen to be submitted before two successive iteration of this script?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're excluding go because we don't want it to be skipped ever. Its worth noting that this would retroactively apply to in flight prs, even if the PR bot has already taken some action on them, so without this exclusion this would break the flow on all in-flight PRs. Functionally, the biggest implication would be that if its been assigned to a non-committer and that non-committer approves it won't get auto-routed to a committer
* Expand pr bot to python * Push reviewer changes * Limit to Python WG + Brian * Add Pablo * Add Yichi * Set start date and format * Feedback
* Expand pr bot to python * Push reviewer changes * Limit to Python WG + Brian * Add Pablo * Add Yichi * Set start date and format * Feedback
This starts running the pr bot on the python label. I did my best to come up with an initial set of reviewers to be round robined to, but it was mostly based on manual inspection. The bot should eventually update that list anyways.
Part of #21417
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
R: @username).CHANGES.mdwith noteworthy changes.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)
See CI.md for more information about GitHub Actions CI.