[bind10-dev] code reviews

[Michael Graff]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 2010-04-16 4:23 PM, JINMEI Tatuya / 神明達哉 wrote:
> At Fri, 16 Apr 2010 15:56:56 -0500,
> Michael Graff <mgraff at isc.org> wrote:
> 
>> I'll admit I've not read too far into what you wrote here.  However, I'm
>> worried we are getting too process-centric on this, and process-centric
>> is somewhat anti-Agile.  Actually it's sometimes very anti-Agile...
> 
> I expected this response:-) But to be clear, it was not my intent to
> propose the constitution of development.  I simply tried to clarify
> what we're "basically" expected to do in more specific way.  The
> actual operation can be flexible, relying on our common sense.

Yea, that's good.  I like general terms rather than hard specifics,
unless there is a need for it.  ISC has always had a "don't make us make
a policy" attitude, and I like that.

> I have one quick question.  If it needs a review on the other hand,
> what should we do?  That's actually my main concern in this
> discussion.  In the strawman idea I proposed to use a short-lived
> branch for this purpose.

It depends on the code.

I've performed a diff (which might even be large) and mailed it around.
 This works if the change is eye-ball observable (like splitting one
flag-type thing into two or more)

I've made a branch.

And I've even used jabber to have someone eye-ball just before or even
just after a commit.

And in some cases, I've needed or wanted more than one reviewer.

I think it also depends on the scope of the change.  I think the
important thing here is to use common sense (or rather, what the group
would think is common sense) and not take anything personally if a
change is bumped up for review.

Plus, there are many reasons to require a review:

  Missing or incomplete tests, or breaking tests

  User-visible design issues (like command or GUI stuff)

  The code is not complete, or is broken (duh!)

  Architectural issues (like, does this scale?)

I think a lot of the review can be done on our list.  Some of that may
be in the ticket, but tickets are short-lived beasts.  I do ask that if
we are discussing a ticket we put it in the email header so we can find
it and match it up.

- --Michael
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.8 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkvI19AACgkQ+NNi0s9NRJ37DACfbGw+eUqwzfwKsTaadjg9sAl2
FrQAoIfIW1d38evBbAZVFZ5u9aR7zakn
=r/gQ
-----END PGP SIGNATURE-----