~alpine/devel

Re: Re: [alpine-devel] RFC: Soft commit rights for established maintainers

Details
Message ID
<20170321103926.-DlBEPtIp%pickfire@riseup.net>
Sender timestamp
1490092766
DKIM signature
missing
Download raw message
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
---
Reply to thread Export thread (mbox)