On Sun, Jul 26, 2015 at 2:58 PM, Paul Moore wrote:

On 25 July 2015 at 20:28, Robert Collins wrote:

...

Those charts doesn't show patches in 'commit-review' - http://bugs.python.org/issue?%40columns=title&%40columns=id&stage=5&%40columns=activity&%40sort=activity&status=1&%40columns=status&%40pagesize=50&%40startwith=0&%40sortdir=on&%40action=search

There are only 45 of those patches.

AIUI - and I'm very new to core here - anyone in triagers can get patches up to commit-review status.

I think we should set a goal to keep inventory low here - e.g. review and either bounce back to patch review, or commit, in less than a month. Now - a month isn't super low, but we have lots of stuff greater than a month.

I'm not actually clear what "Commit Review" status means. I did do a quick check of the dev guide, and couldn't come up with anything,

https://docs.python.org/devguide/triaging.html#stage

On 26 July 2015 at 16:39, Berker Peksağ wrote:

...

I'm not actually clear what "Commit Review" status means. I did do a quick check of the dev guide, and couldn't come up with anything,

https://docs.python.org/devguide/triaging.html#stage

Thanks, I missed that. The patches I checked seemed to have been committed and were still at commit review, though. Doesn't the roundup robot update the stage when there's a commit? (Presumably not, and people forget to do so too). Paul

On Sun, 26 Jul 2015 22:59:51 +0100, Paul Moore wrote:

On 26 July 2015 at 16:39, Berker Peksağ wrote:

...

...

I'm not actually clear what "Commit Review" status means. I did do a quick check of the dev guide, and couldn't come up with anything,

https://docs.python.org/devguide/triaging.html#stage

Thanks, I missed that. The patches I checked seemed to have been committed and were still at commit review, though. Doesn't the roundup robot update the stage when there's a commit? (Presumably not, and people forget to do so too).

Yes, it is manual. Making it automatic would be nice. Patches accepted :) Writing a Roundup detector for this shouldn't be all that hard once you figure out how detectors work. See: http://www.roundup-tracker.org/docs/customizing.html#detectors-adding-behavi... The steep part of the curve there is testing your work, which is something some effort has been made to simplify, but unfortunately I'm not up on the details of that currently. In the meantime, this is a service triagers could perform: look at the commit review issues and make sure that really is the correct stage. Now, as for the original question: First, a little history so that the old timers and the newer committers are on the same page. When 'commit review' was originally introduced, what it was used for was for what was then a "second committer" required review during the RC phase. I have recently (last two years?) with agreement of the workflow list and with no objection from committers shifted this to the model documented in the devguide currently. However, I agree that what is currently in the devguide is not sufficient. Here is my actual intent for the workflow: 1) Issue is opened. Triager/committer sets it to 'patch needed' if they believe the bug should be fixed/feature implemented. (A committer may choose to override a triager decision and close the bug, explaining why for the benefit of the triager and all onlookers.) 2) Once we have a patch, one or more triage or committer people work with the submitter or whoever is working on the patch (who may have no special role or be a triager or be a committer) in a patch review-and-update cycle until a triager or a committer thinks it is ready for commit. 3) If the patch was submitted and/or worked on by a committer, the patch can be committed. 4) If the patch is not by a committer, the stage should be set to 'commit review' by a triager. Committers with time available should, as Robert suggests, look for patches in 'commit review' status *and review them*. The wording of "a quick once over" in the devguide isn't *wrong*, but it does give the impression the patch is "ready to commit", whereas the goal here is to review the work of the *triager*. If the patch is not actually commit ready for whatever reason, it gets bounced back to patch review with an explanation as to why. 5) Eventually (hopefully the first time or quickly thereafter most of the time!) the patch really is ready to commit and gets committed. An here, to my mind, is the most important bit: 6) When the patches moved to commit ready by a given triager are consistently committable after the step 4 review, it is time to offer that triager commit privileges. My goal here is to *transfer* the knowledge of what makes a good review process and a good patch from the existing committers to new committers, and therefore acquire new committers. Now, the problem that Paul cites about not feeling comfortable with the *commit* process is real (although I will say that at this point I can go months without doing a commit and I still remember quite clearly how to do one; it isn't that complicated). Improving the tooling is one way to attack that. I think there can be two types of tooling: the large scale problem the PEPs are working toward, and smaller scale helper scripts such as Paul mentions that one or more committers could develop and publish (patchcheck on steroids). Before that, though, it is clear that the devguide needs a "memory jogger" cheat sheet on how to do a multi-branch commit, linked from the quicklinks section. So, I'm hoping Carol will take what I've written above and turn it into updates for the devguide (assuming no one disagrees with what I've said :) --David

On 7/26/15 6:37 PM, R. David Murray wrote:

On Sun, 26 Jul 2015 22:59:51 +0100, Paul Moore wrote:

...

On 26 July 2015 at 16:39, Berker PeksaÄŸ wrote: So, I'm hoping Carol will take what I've written above and turn it into updates for the devguide (assuming no one disagrees with what I've said :)

David, I'm happy to try to incorporate your detailed steps into the devguide.

Robert Collins writes:

However - 9 isn't a bad number for 'patches that the triagers think are ready to commit' inventory.

So yay!. Also - triagers, thank you for feeding patches through the process. Please keep it up :)

If I were a cheerleader I would be able to lead a rousing “Yay, go team backlog burners!” -- \ “I may disagree with what you say, but I will defend to the | `\ death your right to mis-attribute this quote to Voltaire.” | _o__) —Avram Grumer, rec.arts.sf.written, 2000-05-30 | Ben Finney

On 8/18/15 7:52 AM, R. David Murray wrote:

On Tue, 18 Aug 2015 12:47:22 +1000, Ben Finney wrote:

...

Robert Collins writes:

...

However - 9 isn't a bad number for 'patches that the triagers think are ready to commit' inventory.

So yay!. Also - triagers, thank you for feeding patches through the process. Please keep it up :) If I were a cheerleader I would be able to lead a rousing “Yay, go team backlog burners!” Which at this point in time I think pretty much means Robert, who I also extend a hearty thanks to. (I think I moved one issue out of commit review because the test didn't fail, and that's been it for me since Robert started his burn down...)

--David

Thank you Robert, David, and Ben :D Is anyone game for setting another goal to tackle a targeted subset of patches in "patch review" and move them to "commit review"?