Subject: Stale MPs! Review discussions and Mailing Lists!

From: Robert Carr <[email-masked]>
Date: Mon, 14 Apr 2014 09:28:57 -0700

Today we had a discussion on long pending MPs, review culture, etc...

Kevin recorded some conclusions here:
https://docs.google.com/a/canonical.com/document/d/1IqCx6vejB2yl1l9UWBjufy5UKo3saLlUn1NKmY_6Nd0/edit

One of the takeaways I think is we need to improve bandwidth of
communication on open issues. Meetings are useful but never work fully
across timezones for us and IRC is very lossy...so I'm going to make a
consciousness attempt to use our mailing list more frequently for open
discussions.

Caught myself saying this in IRC and thought in the spirit of self
reference I would post it to the mailing list ;)

---

From: Daniel van Vugt <[email-masked]>
Date: Tue, 15 Apr 2014 09:58:44 +0800

I was wondering how long it would take for someone else to raise this 
issue, not wanting to be the first to complain... ;)

I think the simple best practice is: Check all reviews you are involved 
in every day or so. If you can't keep up with that then abstain or set WIP.

- Daniel

---

From: Kevin DuBois <[email-masked]>
Date: Thu, 17 Apr 2014 11:20:59 -0700

On Mon, Apr 14, 2014 at 6:58 PM, Daniel van Vugt <
[email-masked]> wrote:

> I think the simple best practice is: Check all reviews you are involved in
> every day or so. If you can't keep up with that then abstain or set WIP.
>

I think there are other things that can improve the situation too. My two
(three) cents:

1) I don't know what the distinction between 'needs fixings' vs
'disapproves' means. Both entail some responsibilities and communication
between the blocker and the author. It is sometimes unclear what it would
take to remove a blocking comment. A blocking comment that comes with a
clear route to unblock the objection is much more helpful than 'don't like
it!' This is particularly helpful with naming

2) We should use/cite the style guide more, and codify the conventions in
the code so its clear how to do certain things. As a code writer, I just
want to follow the conventions of the team. As a reviewer, I just want to
keep the code that I'm reviewing consistent with the code base / style
guide.

If someone chooses a different style than the style guide, that should get
a needs fixing.
If someone chooses a different style than a reviewer prefers (and the style
guide is silent/ambiguous), lets debate about style preferences as a team
so we can move forward on the minor points and not have to keep revisitng
the sore spots as blocking 'needs fixings'

3) We should be mindful of our public interfaces. Changing things in
include/ should get the same level of scrutiny that a change to mir's
client api should get. We are somewhat aware of this due to the pain it
causes downstream/in the silos, but as reviewers, we really should make
sure that the public interfaces are good interfaces; interfaces we'd like
to use as a 3rd party.

Cheers,
Kevin