This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: Tracking patch pings
- From: "Joseph S. Myers" <joseph at codesourcery dot com>
- To: Carlos O'Donell <carlos at redhat dot com>
- Cc: <libc-alpha at sourceware dot org>
- Date: Thu, 16 May 2013 00:52:00 +0000
- Subject: Re: Tracking patch pings
- References: <Pine dot LNX dot 4 dot 64 dot 1305152117141 dot 21321 at digraph dot polyomino dot org dot uk> <51941F6A dot 90601 at redhat 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