Review Process Experiment (1 LGTM)

[Julian Edwards]

On Tuesday 20 Aug 2013 09:19:53 Tim Penhey wrote:
> On 20/08/13 01:27, Nate Finch wrote:
> > I accidentally only replied to John with this earlier:
> > 
> > I like the shared responsibility and the review of existing code,
> > but I do lament the loss of that extra pair of eyes on the code.
> > Even in my short time at Canonical, I have almost always seen the
> > second review turn up things the first review missed. This is
> > likely due to each reviewer valuing different things and therefore
> > looking at those things more carefully, and probably also that the
> > first one picked up the easy stuff, so the second one could ignore
> > that and look for more subtle problems. Granted, most of the time
> > it's not incorrect code, just something that could be tweaked to be
> > better, but that still gives us higher quality code overall.
> > 
> > I do think that putting a higher bar on LGTMs is a good thing, and
> > hopefully that can help keep the quality high.
> 
> Well, we have a few things working for us here.
> 
> We use a DVCS which makes reverting a branch that causes real problems
> very easy.  While this doesn't mean we should be committing just
> anything, it does mean that we have an extra back-stop.
> 
> Also, this is just initially a one month experiment, with an end date
> where we evaluate how things went.
> 
> Cheers,
> Tim

I'll add some more things.

Second reviews will nearly always turn up more stuff, because people are different 
and will focus on different things.  So don't get too hung up about it, reverting 
changes is easy.

Also, keep your branches small and focused.  Huge, complex, branches are 
*bound* to have more problems.  Committing small changes at once makes it 
easier to write, easier to review and you get the feel-good factor of landing code 
regularly.

Stay lean and keep up the velocity - code only what you need.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ubuntu.com/archives/juju-dev/attachments/20130820/9749320d/attachment.html>