Review Process Experiment (1 LGTM)

[roger peppe]

This sounds like an excellent plan, thanks!

On 19 August 2013 07:47, John Arbash Meinel <john at arbash-meinel.com> wrote:
> -----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-----
>
> --
> Juju-dev mailing list
> Juju-dev at lists.ubuntu.com
> Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev