Merging pull requests before 72 hours

Chris Jones jonesc at hep.phy.cam.ac.uk
Mon Oct 15 13:16:48 UTC 2018


Hi,

> 1. if the person doing the update is skilled (and at this point I
> generally know the difference),
> 2. if they indicate that they've tested the result,
> 3. if it seems based on history that the listed maintainer is unlikely
> to comment then or ever (and I usually guess right),
> 4. and that if what's being done seems to be a simple version bump

One of the issues is that last point. In general its not possible to be 
100% sure whether or not a 'simple' version bump is a trivial change or 
not, and what the impact of that would be. So either we accept that the 
determination of what is or is not trivial is left to the member that is 
reviewing the PR, at which point we have to accept that not everyone 
will have the same opinion all the time, so sometimes there will be 
disagreements (like the PR that started this), or we have some 'very' 
clear guidelines to follow, which is what I brought up in my last mail.

> that it is okay to merge a request against an openmaintainer port. I
> think I've guessed wrong only a few times in many hundreds of pull
> requests.

Just for the record I have absolutely no problems with the way you have 
been handling things, and am very appreciative of the fact you are 
making the effort to review and merge PRs. More members should really 
help out with this (myself included). So please do not take the above as 
a complaint against what you have been doing. Just a comment that I 
think the guidelines we are working under need clarification.

> If these sorts of things aren't okay to merge pretty quickly, then
> why do we have an openmaintainer designation at all? I mean, if
> there's really no distinction in how you treat an openmaintainer and
> a non-openmaintainer port, why have openmaintainer? Why not just have
> everything closed maintainer?

The way I view openmaintainer is ports that are labeled as such can have 
(non trivial) PRs applied to them, *once the 72 hour timeout has 
expired*, without the explicit consent of the maintainer, as long as 
some sort of agreement of other members that the update is reasonable is 
reached. But the 72 hour timeout should always be adhered to for 
anything that does not classify as 'trivial' (whatever the rules are for 
this).

I do agree though, as Mojca has already brought up, that perhaps we 
should consider getting rid of the 'openmaintainer' tag and instead 
consider everything open by default, unless explicitly flagged as 
'closedmaintainer'. This could also be used as an opportunity to review 
which ports really need to be closed...

Chris

> 
> 
> Perry
> 


More information about the macports-dev mailing list