attn folk doing reviews.

[Martin Pool]

On 24 Jan 2006, John Arbash Meinel <john at arbash-meinel.com> wrote:

> If a main developer submits code, that automatically has one +1, so we
> only need 1 other main developer to review it to get into bzr.dev.
> Should we not be doing this? Should it always be 2 people other than the
> developer who review it before it gets in?
> 
> I don't have a problem if we switch to a slightly more loose definition.
> Where we have patches submitted by people who don't contribute often,
> versus patches from people who have been with us for a long time (like
> Robey, Alexander, etc.).
> Infrequent submitters would need a double +1, while long-term submitters
> would only need a single +1, since they are wise enough not to request a
>  review of code they would not be happy with.
> Alternatively, everyone needs a double +1. But that starts to cause a
> big bottleneck. Since we don't have that many reviewers. (I think we
> have had maybe 6 people do reviews, so require 2 is 1/3rd of all reviewers)
> 
> I've been trying to handle community integration, but I'm only a single
> +1, so stuff like Robey's changes have been falling through the cracks,
> since I was waiting for a second +1.
> 
> I think we could also make a distinction based on how involved the
> change is. If someone is submitting a small bugfix, then a single +1
> could be sufficient (especially if it comes with a test case). Versus
> more involved work.

I think if there is only one +1 after a day or two then there are two
things that can happen.  If you're fairly sure it's the right thing then
go ahead and put it in.  If you want a second opinion, then feel free to
prod the person or people who should do it.  (I tend to read mail which
has me directly in the To list at a higher priority, or you can ask for
another review in the reply.)

It does seem to give good results when we have multiple reviewers as
they consider different aspects.  But getting in with one good +1 after
a couple of days still has some scrutiny and avoids things getting
dropped.

And I really would encourage anyone interested to review code.  Every
vote has weight.  It's a good way to shape the project and learn more
about the code.

-- 
Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060125/02d9d2de/attachment.pgp