X-Original-To: alpine-devel@lists.alpinelinux.org Received: from mx1.riseup.net (mx1.riseup.net [198.252.153.129]) by lists.alpinelinux.org (Postfix) with ESMTP id 36E0D5C32F4 for ; Tue, 21 Mar 2017 10:39:39 +0000 (GMT) Received: from cotinga.riseup.net (unknown [10.0.1.164]) (using TLSv1 with cipher ECDHE-RSA-AES256-SHA (256/256 bits)) (Client CN "*.riseup.net", Issuer "COMODO RSA Domain Validation Secure Server CA" (verified OK)) by mx1.riseup.net (Postfix) with ESMTPS id B24F71A08B3; Tue, 21 Mar 2017 10:39:37 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=riseup.net; s=squak; t=1490092777; bh=EM8aKpnkbxYHT+9Se/gGwbcj4ejIToU6cLhtafu8FFg=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=ZoWQETuUHi4xHW5FyizDOpDnhkkr1DetuoLOYM2PHW34SkdZhyK0gZERcwy+Y6PSF FHtNdzbw75Xu6Xo2KUNYbhhmBLfcCPyjvLxVmTksR35Rz0nf6nKModDqzKuDhaDDYw Op/DvE4mFdgZmTlaqkOGWITsj10umsPxtw70kGzU= Received: from [127.0.0.1] (localhost [127.0.0.1]) (Authenticated sender: pickfire) with ESMTPSA id 1C4A1401EF Date: Tue, 21 Mar 2017 18:39:26 +0800 From: Ivan Tham To: 7heo@mail.com Cc: alpine-devel@lists.alpinelinux.org, nenolod@dereferenced.org Subject: Re: Re: [alpine-devel] RFC: Soft commit rights for established maintainers Message-ID: <20170321103926.-DlBEPtIp%pickfire@riseup.net> References: In-Reply-To: Mail-Followup-To: Ivan Tham , nenolod@dereferenced.org, alpine-devel@lists.alpinelinux.org, 7heo@mail.com X-Mailinglist: alpine-devel Precedence: list List-Id: Alpine Development List-Unsubscribe: List-Post: List-Help: List-Subscribe: 7heo <7heo@mail.com> wrote: > This is a very good idea, but I would like to find a way to get the patches to go through some test of some kind before going in the tree, much in the way PRs on github have to go through travis. > > We already have the testing scripts that jirutka wrote, it would be a shame not to use them. > > Plus that would solve the problem you mentioned at the end of your mail. Theo, can you please wrap the text? > On Tue Mar 21 09:21:11 2017 GMT+0100, William Pitcock wrote: > > As we all know, new contributors to Alpine who wish to become Alpine > > Developers go through a process to gain commit rights, meanwhile > > Maintainers have to find a Developer to commit their work by proxy. > > > > As the project has grown, multiple people have introduced new channels > > for maintainers to work in the distribution, such as submitting > > patches through GitHub. These contribution channels have yielded > > great results for Alpine, resulting in many new maintainers stepping > > forward, some of which have over time gone through the full process > > and gained commit rights. To be quite clear: those initiatives have > > been largely successful. Great idea, having this doesn't require me to bump in a developer with commit access to help to review the patch. > > However, working through the GitHub and Patchwork queues looking for > > packages to commit, I notice that a majority of the commits are just > > generated by abump, which got me thinking... > > > > Is it really worth developer time to go through a bunch of abump > > patches? Is it worth making Alpine users wait for someone to go > > through and commit their changes? In my opinion, most of the time, > > *new* packages are where the review is needed. If the change > > ultimately is just a new SHA512 hash and $version change, then for > > established maintainers, I think it is alright to just accept the > > change into the distribution. And if that is the case, then why not > > allow them to push the change themselves? I think it might not worth the trouble for the maintainer to bump and build it themselves as well. It would be better if [alpine nitro][0] can just try bumping it and probably build it with travis, of course it might as well package it to alpine mirrors. I personally jump bump multiple packages at once and most likely only looked at failed build. Most of the time, package bump just require a minor version change and wouldn't break anything. > > What I propose is quite simple -- we add a git hook to check incoming > > pushes with the following checks: > > > > 1. Is the committer a proper dev? If so, allow the commit through. How would it check for "proper dev"? Through gpg? git commit user name? GitHub user name? What about bumps in Patchwork? > > 2. Is the commit only changing packages (and thusly not configs stored > > in the aports tree)? If not, reject the push. > > > > 3. Are the changed APKBUILDs in question maintained by the person who > > committed the patch (or they have additional privilege, maybe we can > > add a Co-Maintainer field?). If not, reject the push. I think having multiple Maintainer field would be better than having a Co-Maintainer field. > > 4. Are the changed APKBUILDs on a whitelist for the maintainer? If > > not, reject the push. > > > > 5. If all the above checks pass (2 through 4), allow the push. > > > > This would allow us to extend some "soft" commit rights to new > > prospective developers, which in turn gives *positive reinforcement* > > that they are on track to becoming a developer. It also has the > > benefit of reducing the size of the aports patch queues and allowing > > us to concentrate review efforts on people who need those efforts the > > most. > > > > There is some minor risk that folks who have the soft commit rights > > extended to them may from time to time mess up and cause a builder to > > stall. However, in these cases we can simply revert the push or > > correct it. After all, that is what we would be doing with a broken > > GitHub submission. Beyond that, quite honestly, we have all done it > > before anyway *and* if the maintainer breaks something, it gives them > > the possibility of fixing it themselves before it causes any major > > impact. So overall, I would say that it is a very minor risk. [0]: http://pkgs.alpinelinux.org/ -- Do what you like, like what you do! -- Pickfire --- Unsubscribe: alpine-devel+unsubscribe@lists.alpinelinux.org Help: alpine-devel+help@lists.alpinelinux.org ---