Merging pull requests before 72 hours

[Chris Jones]

Hi,

On 16/10/18 13:37, Leonardo Brondani Schenkel wrote:
>> Please point me to where this is documented ? i.e. where is it stated 
>> that openmaintainer allows revision changes. This appears to be one of 
>> the issues here since not everyone, myself included, agrees with you. 
>> The only statement I have found is
>>
>> "If a port's maintainer contains the address <openmaintainer at 
>> macports.org>, this means that the author allows minor updates to the 
>> port without contacting him first. But permission should still be 
>> sought for major changes."
>>
>> which does not mention this point. It leaves the interpretation of 
>> minor and major to the reader.
> 
> Based on your response I had to go to the website and check. To my 
> surprise you're right, I must have imagined or misread the rule and 
> somehow this definition got stuck on my mind.
> 
> To my defense, "minor updates to the port" could be interpreted as 
> either updates to the Portfile or to the software itself, but I must 
> concede that the second interpretation is not clear-cut. Mea culpa for 
> the misleading statement regarding the documentation.

Let me be clear, I was not blaming you for having the view you do, so no 
need to apologize. The point I was making is simply we don't have a 
clear policy on what is and is nor minor, it is left to the 
interpretation of the reader as to what that means, which ultimately 
means confusion on the point.

> 
> But back to the topic at hand, 'openmaintainer' is somewhat of a 
> digression: if the port is not, then PRs have to wait for the 
> maintainer. If the port is, and a PR was opened by a member/committer, 
> then it must follow that the submitter understands that the change does 
> not fall within the definition (otherwise they would have pushed 
> directly) and wants to submit it to the maintainer. In either of these 
> scenarios, a premature merge seems not to be warranted. Doesn't that 
> make sense?

openmaintainer is not a digression in a sense it is tied to what actions 
can be made by members.

- If a port is open maintainer then it is allowed for 'minor' changes to 
be directly committed by members, whilst 'majors' ones require an 
attempt to give the maintainer a chance to comment (which to me means a 
github PR must be opened for that change). The maintainer then has 72 
hours to comment, at which point, under the agreement of those 
commenting on the PR (I would say at least one more person than the 
submitter) it can be merged. The question of what is 'minor' requires 
clarification so we all know what it means... Critical security fixes 
can also be directly merged. Also note that if a third party (non 
member) submits a PR than is determined to be 'minor', then a member 
should be free to merge that PR before the 72 hour timeout, as its 
equivalent to allowing the member to directly commit the changes themselves.

- If a port is not open maintainer then only 'trivial' (e.g. live check 
fixes, comment typos etc.), or urgent security related changes can be 
made directly. Again here I would suggest we need a precise written down 
definition of what this means, just to avoid ambiguities. Everything 
else must go via a PR. If the maintainer fails to respond in some 
reasonable time (what is this ? presumably longer than 72 hours) the 
presumably some other mechanism kicks in, like port abandonment, to 
eventually allow the PR to be merged.

Chris

> 
> // Leonardo.