Review Process Experiment (1 LGTM)

John Arbash Meinel john at arbash-meinel.com
Mon Aug 19 06:47:36 UTC 2013


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

This is the email I want to send to juju-dev, I just want to run it
past everyone to make sure it fits what we agreed to.



One of the things we discussed in the Isle of Man was tweaking our
review process.

We want to try an experiment, where we change how we do code reviews a
little. We want to try it for about a month, and then at the end of it
we evaluate if we want to keep doing it. The short summary is
switching to 1 LGTM for landing patches, and doing a weekly shared
review of parts of the codebase.

The specific changes are:

1) Only one LGTM needed to land code.

   a) This raises the stakes slightly on actually voting LGTM. We'd
      like it to mean "I understand this patch enough that if there was
      a problem, I would be responsible for it."

      It is still great to get comments on a proposal, and have
      conversations about whether you actually understand something.

   b) If you want a specific person to review something you are unclear
      about, *ask* for them to help. If you are unsure it really helps
      to point it out in your merge request overview, rather than
      needing the reviewer to discover it.

   c) This applies for both the submitter and the reviewer. If you want
      a second review, you are encouraged to just ask.

   d) Also, we should feel fine sending comments about something you've
      seen even if that code has already landed.


2) We'll start doing a weekly shared review of existing code.

   a) The idea is that rather than spend resources having 3 people look
      at a new patch (1 writer + 2 LGTM), spend that time auditing the
      code we already have.

      A lot of the benefits from code review are exposing stuff to
      other people, but we only get that exposure for the code that is
      being actively developed, and miss the stuff that you might have
      to maintain later.

   b) In the first pass, we want to give a file for people to look at.
      They should spend about an hour going over it, and then we'll
      spend an hour as a group talking about it. We'll probably take
      one of the daily standup calls for this. I think we'll need to
      split it so that Tim, Ian and Andrew can do one in a
      work-friendly time. We may grow this into a whole package, we're
      just ballparking for how much time this will take.

   c) We expect that this will uncover a fair amount of tech debt. That
      doesn't all need to be fixed immediately, though. If it is
      trivial (can be fixed in the time to review it), just fix it. But
      mostly we want to make sure people understand the code base a bit
      more, and having them be aware that some bits of existing code
      aren't great as a template is still valuable.

So we'd like to switch to this going forward, and re-evaluate if we
feel it has been a positive change on Sept 12th.

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.13 (Cygwin)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iEYEARECAAYFAlIRv4gACgkQJdeBCYSNAAPIDACeL4OBNMyjypq/jKYkSJNOsEhW
mRgAn2WkuFE7oKmo9lS5tw4yoiiSe7cg
=Uz07
-----END PGP SIGNATURE-----



More information about the Juju-dev mailing list