This sounds like an excellent plan, thanks! On 19 August 2013 07:47, John Arbash Meinel <[email protected]> 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 > [email protected] > Modify settings or unsubscribe at: > https://lists.ubuntu.com/mailman/listinfo/juju-dev
-- Juju-dev mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
