openstack team mailing list archive

[Andy Smith <andyster@xxxxxxxxx>]

This stuff was discussed and decided upon in the release meeting a couple
days ago, unless somebody more "official" wants to make a statement the
results of the decision were:

(1) Developer review days are a go, Soren will set up a wiki page assigning
devs to days (i don't think this is done yet).
(2) Corollary is that if you are unable to perform the reviews regularly you
should be dropped from the nova-core group.
(3) A document explaining best practices for the review process will be
generated by Termie (me, not done yet)
(4) Developers should target their reviews to applicable reviewers whenever
possible, much of the developer-on-duty's task will be doing initial review
of un-targeted reviews and re-targeting at an appropriate developer. (jk0
has set up a wiki page to track who has expertise over which areas:
http://wiki.openstack.org/NovaCore , that page may be re-usable as the
developer review day assignment page?)

--andy


On Thu, Feb 24, 2011 at 10:10 AM, Trey Morris <trey.morris@xxxxxxxxxxxxx>wrote:

> +1 to improving reviews since I agree with sandy on it being our greatest
> strength. I also I prefer sandy's approach (unless I'm mistaken), jump in
> and see how it goes, we can update a wiki as things proceed. I don't
> understand the need for a more formal documentation of process. I'd just
> hate for it to be a point of contention.
>
> On Mon, Feb 21, 2011 at 3:34 PM, Andy Smith <andyster@xxxxxxxxx> wrote:
>
>>
>>
>> On Mon, Feb 21, 2011 at 12:52 AM, Thierry Carrez <thierry@xxxxxxxxxxxxx>wrote:
>>
>>> Andy Smith wrote:
>>> > I have some anecdotal evidence to add to this from my time at Google:
>>> >
>>> > (1) At Google in all reality you spent at least 2 days a week pretty
>>> > much only participating in code review and mailing list responses. This
>>> > is due to a couple things, but mostly because code review is taken
>>> > extremely seriously, the reviewer of the code has as much
>>> responsibility
>>> > for what lands as the person writing the code, their name (or names) go
>>> > in change commit. If that code creates a problem it is up to all people
>>> > involved in that process to quickly come up with a resolution.
>>> >
>>> > That responsibility leads to some other great things:
>>> >  * Lessening of self-defensiveness / personal investment in code: the
>>> > code is not yours, it is multiple people's.
>>> >  * You also always have at least one "buddy" who can back up the
>>> > decisions that were made, if you are not around to argue a point that
>>> > person probably can, and no attacks can ever be leveled at you
>>> personally.
>>>
>>> We should definitely add the nova-core reviewers LPids to the commit
>>> message, in order to encourage that spirit. Something like:
>>>
>>> [r=soren,termie] Rest of commit message...
>>>
>>
>> Anybody know how hard it is to incorporate this into our tarmac?
>>
>>
>>>
>>> > (2) At Google you generally have to give explicit targets for who
>>> should
>>> > be your code reviewer. This prevents some tragedy of the commons
>>> > behaviors (when there is nobody assigned everybody expects somebody
>>> else
>>> > to do it).
>>> >
>>> > This also leads to people who are defacto (or explicit) leaders for
>>> > certain sections of code. For example, when fixing a bug on a section
>>> of
>>> > code you are not usually working in it is common to ask around on IRC
>>> > (or just the office) to find out who knows most about that area and
>>> > should do the review.
>>>
>>> This can easily be done by specifying reviewers when you do the branch
>>> merge proposal.
>>>
>>
>> Sure, we just need to remove it automatically selecting "Nova Core" and
>> likewise as a reviewer for you so that people are forced to explicitly add a
>> reviewer.
>>
>>
>>>
>>> > (3) At Google one of the first things that new developers do is read
>>> > through a couple nicely written documents on how to conduct code
>>> > reviews, what your responsibilities are when doing code review, and
>>> some
>>> > ways to make sure your tone comes off constructively.
>>> >
>>> > This keeps everybody on most of the same page and helps acclimatize
>>> > people to social interaction related to coding.
>>>
>>> +1 on reference docs :)
>>>
>>> > I think adopting these behaviors would be in our best interest as a
>>> > project, if that sounds good I am willing to take the time to generate
>>> > the initial draft of the document and get the appropriate
>>> configurations
>>> > / code updated to support tracking reviewers and requiring explicit
>>> > reviewers.
>>>
>>> For (1), ideally Tarmac would support rewriting the commit message in
>>> transit to include those names... (2) is already doable with our current
>>> toolset. For (3), just let us know when the wiki draft is up so that we
>>> can participate in improving it.
>>>
>>> Cheers,
>>>
>>> --
>>> Thierry Carrez (ttx)
>>> Release Manager, OpenStack
>>>
>>> _______________________________________________
>>> Mailing list: https://launchpad.net/~openstack
>>> Post to     : openstack@xxxxxxxxxxxxxxxxxxx
>>> Unsubscribe : https://launchpad.net/~openstack
>>> More help   : https://help.launchpad.net/ListHelp
>>>
>>
>>
>> _______________________________________________
>> Mailing list: https://launchpad.net/~openstack
>> Post to     : openstack@xxxxxxxxxxxxxxxxxxx
>> Unsubscribe : https://launchpad.net/~openstack
>> More help   : https://help.launchpad.net/ListHelp
>>
>>
> Confidentiality Notice: This e-mail message (including any attached or
> embedded documents) is intended for the exclusive and confidential use of the
> individual or entity to which this message is addressed, and unless otherwise
> expressly indicated, is confidential and privileged information of Rackspace.
> Any dissemination, distribution or copying of the enclosed material is prohibited.
> If you receive this transmission in error, please notify us immediately by e-mail
> at abuse@xxxxxxxxxxxxx, and delete the original message.
> Your cooperation is appreciated.
>
>
> _______________________________________________
> Mailing list: https://launchpad.net/~openstack
> Post to     : openstack@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~openstack
> More help   : https://help.launchpad.net/ListHelp
>
>