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: Development suggestions from an ENSIME developer Date: Fri, 22 Jul 2016 16:58:48 +0100 Message-ID: <8760rxvf47.fsf@russet.org.uk> References: <87wpke3kdj.fsf@russet.org.uk> <83h9biyek4.fsf@gnu.org> <877fce3flr.fsf@russet.org.uk> <83d1m6xint.fsf@gnu.org> <83twfhwxeq.fsf@gnu.org> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: ger.gmane.org 1469203287 8672 80.91.229.3 (22 Jul 2016 16:01:27 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Fri, 22 Jul 2016 16:01:27 +0000 (UTC) Cc: emacs-devel@gnu.org To: Stefan Monnier Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Fri Jul 22 18:01:17 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 1bQct3-00066g-21 for ged-emacs-devel@m.gmane.org; Fri, 22 Jul 2016 18:01:17 +0200 Original-Received: from localhost ([::1]:48067 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bQct2-0006e8-7a for ged-emacs-devel@m.gmane.org; Fri, 22 Jul 2016 12:01:16 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:45122) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bQcqk-00065o-QX for emacs-devel@gnu.org; Fri, 22 Jul 2016 11:58:56 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bQcqg-00023G-Ib for emacs-devel@gnu.org; Fri, 22 Jul 2016 11:58:53 -0400 Original-Received: from cloud103.planethippo.com ([31.216.48.48]:45422) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bQcqg-00023A-8c for emacs-devel@gnu.org; Fri, 22 Jul 2016 11:58:50 -0400 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=fQJ46OfNxjmyRy8Ob1ViFdtMNVcj8UCoaDSI8HIeoDQ=; b=FUxkTCmEYyV7oIMV6EOxe1tCup xmDJ4RDt78/EQNDnIpkF974EocavLsUU/CwoWW1+JRvG2m66b1m2yCNHnoLW9xP/UCTancZa6jcaP qKVTZ060V5NiMXczwyuUsAfxmyOZpjwJnvVRFiEKtUnc9N8cKbqZ3X53XF0H7Oks93gqqw1UK2Kb8 p0vdWanjLQAtzKXHGJ+C8IRSkiNiPIPzRcE5yJiQUpxzlMDBV404JWLSj9Q+NN93Wxbzu2PWYIvx4 6tG6iXQvVLqNxGm9qi4GRB+VmUf/Lp2DgK/JtbCcrjiOP8hZGm71XMI+Zo0qYQDz9UkaaLf5p4GU1 pCnXSTAA==; Original-Received: from janus-nat-128-240-225-60.ncl.ac.uk ([128.240.225.60]:2840 helo=russet.org.uk) by cloud103.planethippo.com with esmtpsa (TLSv1.2:DHE-RSA-AES128-SHA:128) (Exim 4.86_1) (envelope-from ) id 1bQcqe-0043eP-TO; Fri, 22 Jul 2016 16:58:48 +0100 In-Reply-To: (Stefan Monnier's message of "Fri, 22 Jul 2016 10:56:50 -0400") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.0.95 (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.21 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" Xref: news.gmane.org gmane.emacs.devel:206042 Archived-At: Stefan Monnier writes: >> For me, most of the work is between 1 and 2. The decision whether the >> patch looks good is often times not an easy one, it requires reading >> more code than is directly affected by the patch, consult the >> documentation, sometimes reading past discussions and bug reports, >> sometimes trying the patched version. That's the most time-consuming >> part of patch review, and it isn't going to go away, nor (it seems) >> will it be helped by the proposed changes in any way. > > It's no silver bullet, no. But the question was not "will it do the > work for me" but just "is it going to be better or worse". > I think it's going to be marginally better. I think neither of your mentioned "not be completely happy, and ask for changes". Being able to do line-by-line comments is a big help. Then the PR'er fixes, squashes. No new patch is needed, just look at the updated state of the world. I found when fiddling with the undo system that sometimes I didn't understand Stefan's comments, because I wasn't sure which bit of code he was talking about (my comprehension I am sure, rather than his explanation). >>> - hopefully/ideally the tool could have already checked copyright.list >>> for you. >> Is there any system that does? I doubt that. > > Out of the box of course not. > But if it's got enough hooks, we should be able to add it ourselves. Copyright.list, no, undoubtly not, but other systems do indeed do CLA signatures and checking. >>> - "3b" can just mark the request as "reviewed by ", so you can give >>> "half-commit" rights to some users. >>> - it sometimes makes it easier for 3rd parties (or for the reviewer) to >>> update the merge request directly (instead of adding comments). >> Does the above really make sense in a project where most patches get >> reviewed by a single person? > > If we want to improve our commit-logs, I think we need to reduce the > number of people who have full commit rights. So while we probably > wouldn't be able to make any use of "half-commit" rights right now, it > could be very useful at some later time. And, actually, removing full commit rights would probably *increase* the number of commiters. I'm still a little nervous when commit to head, because I worry about screwing things up. Submitting a PR is much less of a nervous process. I find this is true even for projects where I do have commit rights. Some projects use PRs even for commiters, so everything goes through two pairs of eyes. Probably not going to be good for all of Emacs, but for some of it, it's going to be good. >>> Of course, there's also the effect on the side of the patch-submitters, >>> where the precise flow is often made easier for them because there's no >>> doubt about where to send the patch, in which format, etc... >> I envision more complaints from submitters about the complexity of the >> process... > > A few old timers might be annoyed, but I'd expect most other submitters > would welcome the change. Especially if people are familiar with it already. Phil