From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: phillip.lord@russet.org.uk (Phillip Lord) Newsgroups: gmane.emacs.devel Subject: Re: Code reviews Date: Tue, 08 Mar 2016 08:48:56 +0000 Message-ID: <878u1tuzgn.fsf@russet.org.uk> References: <56BE7E37.3090708@cs.ucla.edu> <4hd1rw1ubr.fsf@fencepost.gnu.org> <83vb50wxhv.fsf@gnu.org> <87y49vz4cg.fsf@acer.localhost.com> <87vb4zb0i4.fsf@gnu.org> <837fheuu6a.fsf@gnu.org> <877fheb1rh.fsf@wanadoo.es> <87ziua9mwq.fsf@wanadoo.es> <83h9git36k.fsf@gnu.org> <87vb4y9ep9.fsf@wanadoo.es> <83a8mat2aa.fsf@gnu.org> <87r3fm9du0.fsf@wanadoo.es> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: ger.gmane.org 1457426975 12607 80.91.229.3 (8 Mar 2016 08:49:35 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Tue, 8 Mar 2016 08:49:35 +0000 (UTC) Cc: emacs-devel@gnu.org To: Stefan Monnier Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Tue Mar 08 09:49:25 2016 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1adDKU-0004oL-Vh for ged-emacs-devel@m.gmane.org; Tue, 08 Mar 2016 09:49:23 +0100 Original-Received: from localhost ([::1]:33126 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1adDKU-0000JZ-9O for ged-emacs-devel@m.gmane.org; Tue, 08 Mar 2016 03:49:22 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:44865) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1adDKD-0000JS-N5 for emacs-devel@gnu.org; Tue, 08 Mar 2016 03:49:06 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1adDK7-0004U5-Ge for emacs-devel@gnu.org; Tue, 08 Mar 2016 03:49:05 -0500 Original-Received: from cloud103.planethippo.com ([31.216.48.48]:60296) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1adDK7-0004U1-4r for emacs-devel@gnu.org; Tue, 08 Mar 2016 03:48:59 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=russet.org.uk; s=default; h=Content-Type:MIME-Version:Message-ID: In-Reply-To:Date:References:Subject:Cc:To:From; bh=RSlDKaTa0gQkaZKtQO3+Ps/WUdGy1KOtSqOcFdBVJ34=; b=xuCd8hgqvNd+SzKuCOH1kMo48T RgRg+HS8kDn+aZwwo0IJbjyj2+J/J81R13Nz+F3Oq8T4MVUK8lYUaKiDk7IX3nwZfa6mcXHrlEoNU PjnssSIpzv1GBz1h7YBMG8juRJLQdEQFBFNaWNao5bTTv0nPdtFxM/2ex4JvGG+4tvUM2Hk58+BAI t/ZmPNt2odG68+ucGNGy0Ukkiz1G9ZttSVIgpnEayUZiKl1WExz41PVD8umPs87r+TRb622N1Se4w klfhtem7idYluzq8+HiGMG0Ys+38/I338zf35rqSnBUI5TLNLC3V7ySD2YtiHK2G0LnteS3j7T0Pv BAMw1fnA==; Original-Received: from cpc1-benw10-2-0-cust373.gate.cable.virginm.net ([77.98.219.118]:44085 helo=russet.org.uk) by cloud103.planethippo.com with esmtpsa (TLSv1.2:DHE-RSA-AES128-SHA:128) (Exim 4.86) (envelope-from ) id 1adDK5-003NRk-Px; Tue, 08 Mar 2016 08:48:57 +0000 In-Reply-To: (Stefan Monnier's message of "Mon, 07 Mar 2016 22:25:12 -0500") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - cloud103.planethippo.com X-AntiAbuse: Original Domain - gnu.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - russet.org.uk X-Get-Message-Sender-Via: cloud103.planethippo.com: authenticated_id: phillip.lord@russet.org.uk X-Authenticated-Sender: cloud103.planethippo.com: phillip.lord@russet.org.uk X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 31.216.48.48 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:201135 Archived-At: Stefan Monnier writes: >> Introduce code reviews. Don't give commit access to the "golden" >> branches to everyone, just to a few top contributors and reviewers. > > We already have a fair bit of patches submitted and lingering in limbo > forever until someone (almost always the same someone, BTW) finally > loses hope that some of the other contributors take care of it. > > If we could switch to a system where every patch is reviewed before > commit, that'd be great. My own impression is that it will kill the > development pace because too few people are willing to spend the > corresponding efforts. I think that there is a half-way house here. You extensively reviewed the first change that I made to the core of Emacs -- as someone who didn't know the internals or, indeed, even C, there is no way on earth that I could have made that commit without. Other commits are less of problem. Giving people different permissions on different branches seems to make sense. Overtime, people move toward master/emacs-25. Having said that, while I found your feedback and the overall experience very positive, I did feel the lack of a nicer pull/merge request system. Tools such as gitlab or gerrit would have helped. > That's why I've followed a practice of giving out write access very > liberally, with "post-commit spot-check reviews" instead. Indeed, it > means that errors in commit messages can't be fixed (we can fix them in > the ChangeLog files, admittedly, but since I don't use them it doesn't > help me). > > Maybe we could have a half-way system, where commits are pushed to > a branch that is "not fast-forward-only", and this branch is then > auto-merged to the real (fast-forward-only) master branch after a delay > (one day, maybe?) to give time to fix mess ups before they're cast > in stone. I think that this would require some considerable co-ordination. If I push a broken commit to devel branch, and then you fix this commit through rebase, my copy of the branch (and everyone elses) is now broken. I'm pretty sure that this kind of fix up can only happen on a feature branch in a sane way. Phil