Thread: "Claimed" status on Commitfest pages

I see that Brendan has proposed the following definition on
CommitFest:Help:

Reviewing items

If you are reviewing an item, let the other developers know by setting
the claimed parameter of the patch to your name. For example, if you
started reviewing the following patch{{patch|abcd@mail.domain|Add bell and/or whistle|Some Guy}}
you would change it to{{patch|abcd@mail.domain|Add bell and/or whistle|Some Guy|claimed=Some Reviewer}}


I'm not too pleased with this because it suggests that "reviewing" is
something only one person can do at a time.  What I'd like to see is
guidelines more along these lines:

* Short one-or-two-lines comments can be put right into a {{comment}}
item on the commitfest page.

* If you have a more substantive review, post it on the mailing lists
as a reply to the original patch.  Optionally post a link to it as
a {{comment}} on the commitfest page.

* If you are a committer and are taking up the patch for final review
with intent to commit, then mark the commitfest item as "claimed" so
that other committers will not step on your toes.
        regards, tom lane

On Sat, May 10, 2008 at 2:49 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> I see that Brendan has proposed the following definition on
> CommitFest:Help:
>

I wouldn't say I did anything so formal as proposing a definition =)

Someone mentioned that a column to indicate who's handling each patch
would be nice, so I added it.  The comments in :Help were really just
an example to explain how you might use the feature.

I'm not looking to formulate commitfest procedure here (I don't have
quite that level of hubris), just providing some tools to get the job
done more comfortably.

> I'm not too pleased with this because it suggests that "reviewing" is
> something only one person can do at a time.  What I'd like to see is
> guidelines more along these lines:
>

By all means, go ahead and make these changes.  I think your version
makes a lot of sense.

Cheers,
BJ

On Sat, 2008-05-10 at 15:57 +1000, Brendan Jurd wrote:
> On Sat, May 10, 2008 at 2:49 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > I see that Brendan has proposed the following definition on
> > CommitFest:Help:

> I wouldn't say I did anything so formal as proposing a definition =)
> 
> Someone mentioned that a column to indicate who's handling each patch
> would be nice, so I added it.  The comments in :Help were really just
> an example to explain how you might use the feature.
> 
> I'm not looking to formulate commitfest procedure here (I don't have
> quite that level of hubris), just providing some tools to get the job
> done more comfortably.
> 
> > I'm not too pleased with this because it suggests that "reviewing" is
> > something only one person can do at a time.  What I'd like to see is
> > guidelines more along these lines:
> 
> By all means, go ahead and make these changes.  I think your version
> makes a lot of sense.

Certainly having a column mostly set to "nobody" isn't much use. People
want to help, but that's hard when we have no way of knowing which
patches might be ones that are considered open to general review and
which ones are Tom-only.

I added my name to a few patches only to assist with distribution of
work, so people don't waste time, not to "claim them". I figured the
only reason others hadn't done it was 'cos the coding the Wiki was
slightly fiddly.

Completely agree that there isn't/shouldn't be a single reviewer on an
item, but realistically if one of us is obviously reviewing something,
people will work on others while they do that. After that initial review
others join in to check quality. We probably do need to mark them in
some way to say "I am in the process of reviewing this", so we don't
duplicate effort (by mistake).

--  Simon Riggs 2ndQuadrant  http://www.2ndQuadrant.com