[Python-Dev] Pull requests now have labels specifying what stage they are at (original) (raw)
Brett Cannon brett at python.org
Fri Aug 11 17:47:36 EDT 2017
- Previous message (by thread): [Python-Dev] socketserver ForkingMixin waiting for child processes
- Next message (by thread): [Python-Dev] Git and Mercurial Security Update
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
I just pushed a change to Bedevere to help track what stage a pull request is in. The stages are:
- awaiting review
- awaiting core review
- awaiting changes
- awaiting change review
- awaiting merge
The "review" stage is for when a pull request has no reviews either approving or requesting changes (this only applies to PRs not by a core dev; all PRs created by a core dev are flagged as awaiting merge immediately).
The "core review" stage is when a review has been done by one or more non-core folk, but no core devs. The idea with the distinction is to encourage people to help in doing initial reviewing of PRs who happen to not be core devs since triaging issues and reviewing pull requests is the biggest chunk of work in maintaining CPython.
The "changes" stage occurs when a core dev requests changes to a PR (a non-core dev review will not trigger this). A comment will be left to this effect with instructions for the PR creator to leave a comment saying "I didn't expect the Spanish Inquisition" to signal when they believe they are done addressing the reviewer's comments (there is also appropriate comments to explain the quote along with an easter egg(s) :) .
The "change review" label means that the PR creator left the Spanish Inquisition quote and so now the core dev(s) who requested changes should re-review the PR. There will be a comment mentioning the core devs to this effect so that they will get a proper GitHub notification and they can check the label to see if the PR creator believes they are ready for another round of review (compared to now where you have to hunt around in the PR to see if it's ready for another review).
The "merge" label means the PR has received an approving review from a core dev and so it should be ready to go.
Now when any state change happens to trigger these labels, any preexisting "awaiting" label will be cleared and the new one set. But removing or setting a label manually won't trigger an action, so in this instance you can override Bedevere manually until some later event occurs which triggers an automatic update.
One thing I do want to call out is that it's now more important that core devs leave appropriate "approved" and "request changes" reviews through the GitHub UI instead of just leaving a comment when they approve or want changes to a PR, respectively. Not only is this necessary to automate this part of the stage labeling, but it also visibly exposes in the GitHub UI the review state of a PR.
I'm not backfilling the open PRs as it's more work and I don't want to do said work. ;) But as PRs are opened, reviewed, etc., things should slowly trickle their way through the various PRs.
I should mention this is the second-to-last "intrusive" workflow change I have planned to introduce some new aspect to the workflow (the last one being a status check for a news entry after every branch has been blurbified). After these two changes land, everything else I have planned is just touching things up (e.g. the CLA check becoming a status check instead of a label, touching up CONTRIBUTING.md, etc.). I think some other people have plans to improve things as well, but at least my TODO list of "radical" changes to our workflow is almost done! -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://mail.python.org/pipermail/python-dev/attachments/20170811/10c12793/attachment.html>
- Previous message (by thread): [Python-Dev] socketserver ForkingMixin waiting for child processes
- Next message (by thread): [Python-Dev] Git and Mercurial Security Update
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]