Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 28 Nov 2012 23:02:34 -0800
From:      "Simon J. Gerraty" <sjg@juniper.net>
To:        Alfred Perlstein <bright@mu.org>
Cc:        "src-committers@freebsd.org" <src-committers@FreeBSD.org>, Andre Oppermann <andre@FreeBSD.org>, Peter Wemm <peter@wemm.org>, Garrett Cooper <yanegomi@gmail.com>, "svn-src-all@freebsd.org" <svn-src-all@FreeBSD.org>, "Robert N. M. Watson" <rwatson@FreeBSD.org>, "svn-src-head@freebsd.org" <svn-src-head@FreeBSD.org>
Subject:   Re: Reviewing before commit and stability minibikeshed (Re: svn commit: r243627 - head/sys/kern)
Message-ID:  <20121129070234.9C68458094@chaos.jnpr.net>
In-Reply-To: <50B6E1DD.2030908@mu.org>
References:  <201211272004.qARK4qS8047209@svn.freebsd.org> <CAGE5yCpxOdsjefe6quR_gjs82pk9a2e_H_WUNUWhUGA3WZPJaw@mail.gmail.com> <50B54180.5020608@freebsd.org> <alpine.BSF.2.00.1211272246560.37292@fledge.watson.org> <50B54492.5040100@freebsd.org> <956CE44A-BA0F-4FE4-AA38-F4B90C85ECBA@FreeBSD.org> <50B54CE0.6080008@freebsd.org> <2A12C740-1D72-4D30-B663-47A37AAC2FF3@FreeBSD.org> <50B5C4F1.1020002@freebsd.org> <50B64C43.50001@mu.org> <E11B3FAA-440E-492A-A8DA-FFD376E9B176@gmail.com> <50B6E1DD.2030908@mu.org>

next in thread | previous in thread | raw e-mail | index | archive | help

>On 11/28/12 7:49 PM, Garrett Cooper wrote:
>> I know it's sort of done in some groups [based on commit messages], but it w
>ould be nice to have it better formalized and socialized as well. Things like 

Trying to get too formalized could be counter productive.

>> An extension of this code review idea would probably be reviewboard. Email b
>ased review is doable and a lot of OSS groups use it, but there are some nice 
>points to using a more advanced tool, in particular:
>> 1. Colorized diffs.

FWIW all the Colorized tools I've seen just hurt my eyes.

On Wed, 28 Nov 2012 20:17:33 -0800, Alfred Perlstein writes:
>I've seen what happens with large groups, it doesn't scale and basically 
>you wind up with the following type of reviewers:

The issues you cite, are the result of taking a good idea too far.
Mandating reviews for all changes - does not make sense, and all the
ills you describe are natural consequences.

But AFAIK that wasn't what Robert was talking about.

Some bits of code do warrant extra care when touching.
Unless you are the sole author and maintainer (and sometimes even then)
getting a 2nd opinion (from someone familiar with the code) makes sense.

That does not have to map to a requirement for formal reviewers, review
tracking etc - none of which would scale in a volunteer organization
anyway.




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20121129070234.9C68458094>