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