Burning down the backlog.
On 21 July 2015 at 19:40, Nick Coghlan <ncoghlan@gmail.com> wrote:
All of this is why the chart that I believe should be worrying people is the topmost one on this page: http://bugs.python.org/issue?@template=stats
Both the number of open issues and the number of open issues with patches are steadily trending upwards. That means the bottleneck in the current process *isn't* getting patches written in the first place, it's getting them up to the appropriate standards and applied. Yet the answer to the problem isn't a simple "recruit more core developers", as the existing core developers are *also* the bottleneck in the review and mentoring process for *new* core developers.
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. For my part, I'm going to pick up more or less one thing a day and review it, but I think it would be great if other committers were to also to do this: if we had 5 of us doing 1 a day, I think we'd burn down this 45 patch backlog rapidly without significant individual cost. At which point, we can fairly say to folk doing triage that we're ready for patches :) -Rob -- Robert Collins <rbtcollins@hp.com> Distinguished Technologist HP Converged Cloud
On 25 July 2015 at 20:28, Robert Collins <robertc@robertcollins.net> 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, but looking at the first issue on that list (http://bugs.python.org/issue24585) the change has been committed but it can't practically be tested until it's released in a beta. The second on the list also seems to have been committed. While post-commit reviews are useful, I wouldn't classify not getting them as a pressing problem (after all, the change is in, so in the worst case we'll get bug reports if there *were* any issues). Getting patches to a state where they can be committed, and more importantly actually committing them, is the bigger problem. Looking at "Issues with patches" for example, I find http://bugs.python.org/issue21279. That is a simple doc patch, and there's a pretty lengthy discussion on getting the exact wording right (plus six revisions of the patch). That seems excessive, but nevertheless... My problem is that in order to commit that patch (which seems to be the next step - I see no reason not to) I'd need to go through working out all of the commit/merge process, and make sure I got it all right. That's a lot of work (and some level of risk) - particularly compared to working on pip, where I hit the "merge" button, and I'm done. So that patch languishes until someone other than me, who's more familiar with the commit process, merges it. Of course, I could learn the patch process, but my time for working on tracker items is limited, and my brain is getting old, so the likelihood is that I'll forget some of the details before next time, so that learning time isn't a one-off cost. This is basically what the improved dev workflow peps are about, so people are working on a solution, but to me that's the big problem - we need a big red "Commit" button that provides a zero-effort means for a core dev to point at a patch on the tracker that's already been fully reviewed, and just say "do it". Personally, if I were actually expecting to do enough commits to justify the effort, I'd write a script to do that (and I'd probably isolate my build environment in a VM, somehow, as I rebuild my main PC often enough that even having a build env present on my PC isn't a given). Paul
On Sun, Jul 26, 2015 at 2:58 PM, Paul Moore <p.f.moore@gmail.com> wrote:
On 25 July 2015 at 20:28, Robert Collins <robertc@robertcollins.net> 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,
On Sun, Jul 26, 2015 at 11:39 AM, Berker Peksağ <berker.peksag@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,
What is probably missing from the dev-guide is an explanation that stages do not necessarily happen in the linear order. For example, a committer may reset the stage back to "needs a patch" if the patch does not pass a "commit review".
On 26 July 2015 at 16:39, Berker Peksağ <berker.peksag@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,
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 26/07/2015 22:59, Paul Moore wrote:
On 26 July 2015 at 16:39, Berker Peksağ <berker.peksag@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,
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
I wouldn't know. I certainly believe that the more time we spend assisting Cannon, Coghlan & Co on the core workflow, the quicker, in the medium to long term, we put the backlog of issues to bed. -- My fellow Pythonistas, ask not what our language can do for you, ask what you can do for our language. Mark Lawrence
On Sun, 26 Jul 2015 22:59:51 +0100, Paul Moore <p.f.moore@gmail.com> wrote:
On 26 July 2015 at 16:39, Berker Peksağ <berker.peksag@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,
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 27 July 2015 at 11:37, R. David Murray <rdmurray@bitdance.com> wrote:
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.
+1000 :) A few years back, I wrote this patch review guide for work: https://beaker-project.org/dev/guide/writing-a-patch.html#reviewing-a-patch Helping to create a similarly opinionated guide to assist reviewers for CPython has been kicking around on my todo list ever since, but it's a lot easier to create that kind of guide when you're the appointed development lead on a relatively small project produced almost entirely through paid development - I wrote it one afternoon, uploaded it to gerrit.beaker-project.org, and the only folks I needed to get to review it were the other full-time developers on the Beaker team. I don't think that would be the right way to create such a guide for a volunteer driven project like CPython, but steering a more collaborative community discussion would require substantially more time than it took me to put the Beaker one together. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
On 7/26/15 6:37 PM, R. David Murray wrote:
On Sun, 26 Jul 2015 22:59:51 +0100, Paul Moore <p.f.moore@gmail.com> wrote:
On 26 July 2015 at 16:39, Berker PeksaÄŸ <berker.peksag@gmail.com> 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.
On 26 July 2015 at 07:28, Robert Collins <robertc@robertcollins.net> wrote:
On 21 July 2015 at 19:40, Nick Coghlan <ncoghlan@gmail.com> wrote:
All of this is why the chart that I believe should be worrying people is the topmost one on this page: http://bugs.python.org/issue?@template=stats
Both the number of open issues and the number of open issues with patches are steadily trending upwards. That means the bottleneck in the current process *isn't* getting patches written in the first place, it's getting them up to the appropriate standards and applied. Yet the answer to the problem isn't a simple "recruit more core developers", as the existing core developers are *also* the bottleneck in the review and mentoring process for *new* core developers.
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.
For my part, I'm going to pick up more or less one thing a day and review it, but I think it would be great if other committers were to also to do this: if we had 5 of us doing 1 a day, I think we'd burn down this 45 patch backlog rapidly without significant individual cost. At which point, we can fairly say to folk doing triage that we're ready for patches :)
We're down to 9 such patches, and reading through them today there are none that I felt comfortable moving forward: either their state is unclear, or they are waiting for action from a *specific* core. 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 :) -Rob -- Robert Collins <rbtcollins@hp.com> Distinguished Technologist HP Converged Cloud
Robert Collins <robertc@robertcollins.net> 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 Tue, 18 Aug 2015 12:47:22 +1000, Ben Finney <ben+python@benfinney.id.au> wrote:
Robert Collins <robertc@robertcollins.net> 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
On 8/18/15 7:52 AM, R. David Murray wrote:
On Tue, 18 Aug 2015 12:47:22 +1000, Ben Finney <ben+python@benfinney.id.au> wrote:
Robert Collins <robertc@robertcollins.net> 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"?
On Mon, Aug 17, 2015 at 7:37 PM, Robert Collins <robertc@robertcollins.net> wrote:
On 26 July 2015 at 07:28, Robert Collins <robertc@robertcollins.net> wrote:
For my part, I'm going to pick up more or less one thing a day and review it, but I think it would be great if other committers were to also to do this: if we had 5 of us doing 1 a day, I think we'd burn down this 45 patch backlog rapidly without significant individual cost. At which point, we can fairly say to folk doing triage that we're ready for patches :)
We're down to 9 such patches, and reading through them today there are none that I felt comfortable moving forward: either their state is unclear, or they are waiting for action from a *specific* core.
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 :)
Awesome! If you're looking for something to do, the change in this patch had broad consensus, but has been stalled waiting for review for a while, and the lack of a final decision is leaving other projects in a somewhat uncomfortable position (they want to match CPython but CPython isn't deciding): https://bugs.python.org/issue24294 ;-) -n -- Nathaniel J. Smith -- http://vorpus.org
participants (10)
-
Alexander Belopolsky -
Ben Finney -
Berker Peksağ -
Carol Willing -
Mark Lawrence -
Nathaniel Smith -
Nick Coghlan -
Paul Moore -
R. David Murray -
Robert Collins