Merging pull requests before 72 hours

Rainer Müller raimue at macports.org
Thu Oct 18 18:24:12 UTC 2018


On 2018-10-17 09:42, Christopher Jones wrote:
> On 17 Oct 2018, at 1:17 am, Rainer Müller <raimue at macports.org> wrote:
>>
>> What you describe would be the process without openmaintainer. With
>> openmaintainer, "minor updates" can be pushed directly by other project
>> members.
>>
>> I would always assume that a project member was looking for explicit
>> maintainer approval when submitting a change to an openmaintainer port
>> as a pull request. Otherwise they could just have pushed it directly.
>> Therefore I would recommend not to merge them unless approved by the
>> port's maintainer.
> 
> What then about non-member git PRs, should they wait the 72 hours before merging or not ? Non-members have no othert option other than submit a PR (or a ticket).
> If a member reviews one of these with the 72 hours and deems it ‘minor’, surely to be consistent with the
> fact they could have just committed that change directly themselves, they should be free to merge it immediately ?
> 
> Allowing members to directly commit ‘minor’ changes directly but then complaining when ‘minor’ PRs get merged quickly to me is inconsistent.

I see your point. However, in my opinion, merging changes by others is
different from pushing your own changes.

I would say PRs should always wait for 72 hours for approval by the
maintainer, regardless of who opened them. Of course the usual
exceptions apply, like the port is broken, has a security issue, or
affects a lot of dependents.

>> The problem has always been that the definition of what is considered a
>> minor or a major change largely depends on the software we are
>> packaging. It is hard to find a more specific rule that still applies
>> universally to all ports.
> 
> That is understood. Either we allow ‘minor’ version updates directly, and accept that there will always be disagreements on what this means exactly, and sometimes a ‘minor’ update will be made that turns out not to be so minor, or we don’t allow any version updates under the ‘minor’ tag, and accept the additional work as members that *every* update has to then go via a PR. The guide also could perhaps be improved to be a bit more explicitly on what minor means. For instance, if we agree that includes minor version updates, then that should be stated.

Okay, then we should clarify that...
Any suggestion on how it should be phrased?

Rainer


More information about the macports-dev mailing list