On Mon, Feb 21, 2011 at 12:52 AM, Thierry Carrez <thie...@openstack.org>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@lists.launchpad.net > Unsubscribe : https://launchpad.net/~openstack > More help : https://help.launchpad.net/ListHelp >
_______________________________________________ Mailing list: https://launchpad.net/~openstack Post to : openstack@lists.launchpad.net Unsubscribe : https://launchpad.net/~openstack More help : https://help.launchpad.net/ListHelp
