From owner-svn-src-head@FreeBSD.ORG Thu Nov 29 07:05:13 2012 Return-Path: Delivered-To: svn-src-head@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 9C12C7D4; Thu, 29 Nov 2012 07:05:13 +0000 (UTC) (envelope-from sjg@juniper.net) Received: from exprod7og108.obsmtp.com (exprod7og108.obsmtp.com [64.18.2.169]) by mx1.freebsd.org (Postfix) with ESMTP id 0554B8FC0C; Thu, 29 Nov 2012 07:05:09 +0000 (UTC) Received: from P-EMHUB03-HQ.jnpr.net ([66.129.224.36]) (using TLSv1) by exprod7ob108.postini.com ([64.18.6.12]) with SMTP ID DSNKULcJJDMRa77b0Wqe+tbHYfoyBH3i4MFD@postini.com; Wed, 28 Nov 2012 23:05:13 PST Received: from magenta.juniper.net (172.17.27.123) by P-EMHUB03-HQ.jnpr.net (172.24.192.33) with Microsoft SMTP Server (TLS) id 8.3.213.0; Wed, 28 Nov 2012 23:02:35 -0800 Received: from chaos.jnpr.net (chaos.jnpr.net [172.24.29.229]) by magenta.juniper.net (8.11.3/8.11.3) with ESMTP id qAT72Y324267; Wed, 28 Nov 2012 23:02:34 -0800 (PST) (envelope-from sjg@juniper.net) Received: from chaos.jnpr.net (localhost [127.0.0.1]) by chaos.jnpr.net (Postfix) with ESMTP id 9C68458094; Wed, 28 Nov 2012 23:02:34 -0800 (PST) To: Alfred Perlstein Subject: Re: Reviewing before commit and stability minibikeshed (Re: svn commit: r243627 - head/sys/kern) In-Reply-To: <50B6E1DD.2030908@mu.org> References: <201211272004.qARK4qS8047209@svn.freebsd.org> <50B54180.5020608@freebsd.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> <50B6E1DD.2030908@mu.org> Comments: In-reply-to: Alfred Perlstein message dated "Wed, 28 Nov 2012 20:17:33 -0800." From: "Simon J. Gerraty" X-Mailer: MH-E 7.82+cvs; nmh 1.3; GNU Emacs 22.3.1 Date: Wed, 28 Nov 2012 23:02:34 -0800 Message-ID: <20121129070234.9C68458094@chaos.jnpr.net> MIME-Version: 1.0 Content-Type: text/plain Cc: "src-committers@freebsd.org" , Andre Oppermann , Peter Wemm , Garrett Cooper , "svn-src-all@freebsd.org" , "Robert N. M. Watson" , "svn-src-head@freebsd.org" X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 29 Nov 2012 07:05:13 -0000 >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.