Re: HEAD is open for 8.5 development - Mailing list pgsql-hackers

On Thu, Jul 2, 2009 at 2:42 AM, Peter Eisentraut<peter_e@gmx.net> wrote:
> On Thursday 02 July 2009 05:29:58 Robert Haas wrote:
>> If you are taking a look at
>> any of the patches submitted for this CommitFest, please put your name
>> next to them on the wiki.  Please ALSO add a comment saying something
>> like "tgl says: I am reviewing this, no need to assign a round-robin
>> reviewer".
>
>> During the last CommitFest, we had occasional shenanigans where
>> someone would stick the name of a committer or other community member
>> next to a patch just because they had made some comments on it.  But
>> the person whose name got stuck there didn't necessarily know about it
>> or feel any responsibility to conduct a complete review.  So it would
>> be helpful if everyone could make sure to add a comment when claiming
>> patches.
>
> Um, how about no one sticks no one's name to any patch unless it is themselves
> or they are the commit fest management team and the reviewer has explicitly
> OK'ed it.

I'm all in favor of that.

> Let's not begin to use several levels of "Reviewer: X", "X says: I will really
> review it.", "RH says: This X is legit.", "Reconfirmed-by: X".  The wiki is
> weird enough to use that we shouldn't need to edit it in more than one place
> for each logical step.

Well, IIRC, Tom was very insistent when we first set this up that the
last column of the chart should be labelled "Reviewers" rather than
"Reviewer", so as not to project the idea that if there already is a
reviewer, then no other reviewers need apply.  And in fact we do
sometimes assign multiple reviewers, if the first one gets busy and
can't continue reviewing or if the first one flakes out and never
posts a review at all or if the patch needs review from somebody else
with a different skillset or if someone who has not volunteered to be
a rrreviewer takes an interest in a patch and volunteers to review
that specific thing or if (ha ha ha) we have enough rrreviewers to
assign extras to certain patches.

So I think it's important to clarify the distinction between when a
committer is planning to look at the patch but maybe somebody else
should too, and when the committer is planning to take exclusive
responsibility for the patch and nobody else should bother touching
it.  Unless, of course, we want to make the rule that if a committer's
name is next to a patch, then that automatically means that no help
from anyone else is needed.  But that seems like a policy that would
inevitably give rise to exceptions.

Also, in general, I have found that when trying to recall the status
of a patch, the comment trail is a lot more useful than the main
summary line.  It's a lot easier to look at the comment trail and try
to remember whether anything has happened "since then" than it is to
look at the at the summary line and try to remember whether it matches
current reality.  Also, if people date-stamp their comments (which
will happen automatically if we get over to my new system), it makes
it possible to get a sense of how long it's been since a certain state
change took place.  Right now, I can look at the Wiki page and say,
"Oh, Peter is looking at a bunch of patches, great".  And I know that
it's recent because your name wasn't there a couple of days ago, and I
don't really care whether you're going to commit them soon because the
CommitFest hasn't even started yet.  But by the first of August I
suspect there will be so much activity that without a comment trail it
will be impossible to reconstruct what is going on.  So I'd like to
encourage not just committers claiming patches but ANYONE who updates
ANYTHING to leave a comment with the details.

....Robert