Re: Tracking patch pings

["Joseph S. Myers" <joseph at codesourcery dot com>]

On Wed, 15 May 2013, Carlos O'Donell wrote:

> What problem are we trying to solve?
> * Keeping track of patches that need review as a release approaches?

Yes (but generally during development, not just as a release approaches, 
although it's good to try to avoid patches going over to another release 
cycle unnecessarily).

> * Make such review as light-weight as possible?

Yes.  Receive email, reply "OK" (or with more detailed comments, quoting 
particular bits of the patch as appropriate), no need for other tools to 
be involved.  Patch and non-patch discussions can transparently turn into 
each other depending on what issues people raise.

> Is using bugzilla too heavy-weight?

Not necessarily, given it's the committer's responsibility to close the 
bug after committing a patch that fixes the bug (and I'm more concerned 
about making things easy for the reviewer, given that each patch has one 
writer and a large number of readers).  But patches don't necessarily have 
a one-to-one correspondence to well-defined bugs; so far, a bug in 
Bugzilla should be about a defect in the library sources, rather than 
about a particular patch that is proposed to fix such a defect - or to do 
something about which there might be no consensus it's a defect at all.

Tagging bugs with submitted patches is one possibility, but again someone 
needs to keep such tags up to date.

> Is using the wiki too unstructured?

Not necessarily.  My wiki suggestion was to list patches only at the point 
of ping, so making it more lightweight than having some special process 
for *every* patch.

> What benefit do people see in patchwork over an unstructured
> wiki list and heavily structured feature bug?

The benefit is that it fails safe - if a patch is submitted and noone 
follows any special process, it's automatically tracked until someone says 
it no longer needs tracking - and imposes no extra work on reviewers, and 
doesn't require any extra work from submitters either.  I think you'll 
need someone who tries to keep the patchwork data clean, but you can also 
encourage contributors to do so themselves for their own patches.

So, if someone is willing to try to keep the patchwork data clean for a 
while, I think we should try patchwork.  That does not exclude also using 
other tools.

> Lately my opinion has begun to lean towards the fact that bugzilla,
> the wiki, and patchwork all lack critical integration with git which
> is required to make the workflow easier.
> 
> Other people think the same thing or gerrit wouldn't exist.
> 
> For a quick introduction to gerrit:
> https://review.typo3.org/Documentation/intro-quick.html

I think that's a lot more (too) heavy-weight than the alternatives.  
Review shouldn't require extra tools or websites, and it's a feature of 
patchwork that they don't.  I'm aware that gerrit can send emails (but 
don't know if they're plain text, sensibly handle compression for large 
patches, will properly quote bits of patches being commented on, etc.) and 
reviews can be done from the command line, but don't see anything obvious 
there about reviewing by email reply, like you can comment on a bug by 
email.

> (b) Reviewers review patches in gerrit, and if you get consensus
>     it gets checked into the master repo.

Consensus requires a human judgement of the submitter based on what 
reviewers have said, which may not be a simple "yes" or "no" and may e.g. 
suggest that the submitter considers a change before committing, or 
indicate a time to wait for other people to comment.  I don't think 
replacing it by automatic commit on some sort of approval is such a good 
idea; I think the submitter should be able to decide that there is 
something wrong with their approved patch so they aren't going to commit 
it - or that they'll postpone a commit if approval comes too soon before 
going away for a while, or if they think the tree is too unstable or they 
want to run some extra tests first, or if they know there might be an 
interaction with another change that doesn't show up as a textual 
conflict.  And they should be able to make such changes as are in the 
circumstances appropriate when preparing the tree for the final commit - 
without needing to go through any sort of review process again if the 
consensus didn't need such a review for the final revisions.

> A patch that needs review, and got it, would get checked in
> right away. There is no intermediate state of patch checked
> in but tracker not updated.

That sort of intermediate state is good to avoid (and git hooks could 
probably help with it for any system).  But I think the intermediate state 
where there is some level of approval and a commit might be OK, but the 
committer has not yet chosen that it's a suitable time to commit, is a 
feature, not a bug.

And the intermediate state that we currently have, that's a problem, is 
patches submitted (quite possibly by people without write access) but not 
reviewed or tracked - and patchwork can reliably address the "not tracked" 
part of that.

-- 
Joseph S. Myers
joseph@codesourcery.com