[bind10-dev] code reviews

[Michael Graff]

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

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 think any of us developers, or ones we hire, can manage to decide if
the change is one of these things:

  Is the change big?

  Is the change functional?

  Is the change likely to be controversial?

If it is any of the above, it is likely grounds for review.  If it is
none of them, or the change was discussed in-depth with others, then it
might be ok to let it through without review.

I think as engineers at ISC we have the responsibility to say, "Hey
there, I think that change is too big to not be reviewed."  If we don't
feel comfortable saying this to each other, then mention it to Shane.

This is all dependent on one feature we cannot skip:  tests.  If we have
a reliable set of tests which cover the new and old code alike, then we
can be much more liberal on letting code in.  If we don't know if new
code is going to break old code or not, then we cannot really risk
anything to go in unchecked, excepting comments.

About committing to the trunk:

I do not think we should commit non-working code to the trunk ever.
There is what I think is a good method called "continuous integration"
where you merge features into the source as soon as they are ready.
This doesn't mean complete if there are many sub-features involved (for
example, "transfer in" and "transfer out" could be part of a "transfer
zones feature" on paper) but at all times the trunk is releasable.

What I mean by releasable is simple:  If Shane decided to do a release
tomorrow, what is on trunk runs, is tested, and is expected to be a good
release.  This keeps a feature from becoming such a huge merge, which is
certainly a bonus.  It also lets us review in parts for some things
rather than a huge review.

It also lets us find bugs MUCH faster, and if releases happen while a
feature is being expanded on, lets our customers provide feedback and
problem reporting earlier.

I'm suggesting we adopt this scheme.  We already do most of this now I
think so it shouldn't be a problem.

CI requires:

  tests (go figure, I sound like a broken record...)
  test reporting (we have some of this)
  a bit of discipline (we have this or can find it :)
  management buy-in (shane? :)

Some more (and actually relevant) info is on
  http://en.wikipedia.org/wiki/Continuous_integration

Note that this sort of precludes branches in many cases, other than the
expected release branches (like 9.5, 9.6, and 9.7 are in the bind 9 world.)

I would like to see BIND 9 get here too.

- --Michael

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

iEYEARECAAYFAkvIzxgACgkQ+NNi0s9NRJ0m+ACfbV1lxtRRhdg4Bc7SJ1nqQWGv
A/gAoK9jGFrVweiXTMY92i9AxJsQWe2g
=dSum
-----END PGP SIGNATURE-----