Merging pull requests before 72 hours
Chris Jones
jonesc at hep.phy.cam.ac.uk
Mon Oct 15 22:24:23 UTC 2018
> On 15 Oct 2018, at 11:18 pm, Chris Jones <jonesc at hep.phy.cam.ac.uk> wrote:
>
> Hi,
>
>> On 15 Oct 2018, at 10:34 pm, Leonardo Brondani Schenkel <lbschenkel at macports.org> wrote:
>>
>> My two cents:
>>
>> I'm a committer, and if I'm doing a trivial bump of an openmaintainer port I'll push it directly.
>
> Depends entirely on what you consider trivial. If you consider a version update a trivial bump then I disagree. This in my opinion is something that should also be submitted as a PR so the port maintainer always gets a chance to review it. If the port is open maintainer and they do not respond within the 72 hour timeout, it can then be merged without their comment, but not before. For a committer to directly commit an update to a port that isn’t trivial and they do not maintain is in my humble opinion an abuse of the member role. Members should not bypass the PR review stage just because they can.
>
>> If I'm opening a GitHub PR for an openmaintainer port this means that for some reason I want the maintainer's opinion/review before it gets merged.
>
> In my opinion the ‘rules’ for when this happens should not be subjective, as they currently seem to be. There needs to be a clear policy on what is and is not trivial and thus what does and does not require PR review.
>
>> As a maintainer, I would be annoyed if I was notified of a PR and at the time I look into it (within the 72-hour period) it was already merged.
>
> Agreed. I also would expect to be always given the opportunity to review a PR for any non trivial update, and not for another maintainer to just directly commit it by passing the review stage.
Correction. .... and not for another MEMBER to just directly commit it by passing the review stage.
>
>> My belief is that in general nobody else should merge the PRs besides the submitter of the PR and/or the maintainer.
>
> Disagree. Any member with commit rights should be able merge PRs. As long as the procedures have been followed, so either the maintainer has oked the changes, or the 72 hour timeout has expired and another reviewer has agreed with the changes, it does not really matter who finally clicks merge.
>
> ( Its actually quite common practise in many circles for submitters of a PR to require third party approval, and for someone else to merge it, and for the submitter to simply do it themselves to be discouraged. )
>
>> They can merge at any time of their choosing in case they want to proceed, and there is no need to second guess their intentions.
>
> Only the maintainer of a port should be free to merge before the 72 hour timeout, if they have the rights to do so. If they do not have commit rights, then can signify they have agreed to the changes at which point any member could merge.
>
> Just my views... take it or leave it.
>
> Chris
>
>>
>> So I'm with Ryan and Mojca on this one.
>>
>> // Leonardo.
>
More information about the macports-dev
mailing list