[Python-Dev] Burning down the backlog.

[R. David Murray]

On Sun, 26 Jul 2015 22:59:51 +0100, Paul Moore <p.f.moore at gmail.com> wrote:
> On 26 July 2015 at 16:39, Berker PeksaÄŸ <berker.peksag at gmail.com> 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-behaviour-to-your-tracker

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