[Matplotlib-devel] Proposal to amend the PR merge rules

[Paul Hobson]

I think this would be a good policy as well. Can I get some clarification
on the flow? The way I understand it:

1) Someone (committer, contributor, or first-timer) opens a simple PR
2) Committer A reviews it, adds a "single-review-merge" label, then
digitally walks away
3) No less than two weeks later, Committer B sees the PR, notices the
label, opens it up, and merges without reviewing.

Does that capture a successful "single-review-merge" lifecycle?
-paul

On Tue, Jan 15, 2019 at 9:57 AM Nelle Varoquaux <nelle.varoquaux at gmail.com>
wrote:

> Hello,
>
> Overall, I think this is a good idea, but I would like specifics on what
> "uncontroversial but uninteresting" PR mean. IMO, that should exclude any
> PR with API changes.
>
> Cheers,
> N
>
> On Tue, 15 Jan 2019 at 06:00, Antony Lee <anntzer.lee at gmail.com> wrote:
>
>> Hi all,
>>
>> During the weekly dev call, I proposed to amend the PR merge rules,
>> following an initial comment on Github [
>> https://github.com/matplotlib/matplotlib/pull/13173#issuecomment-453921220].
>> There was general agreement among the devs present (Tom, Hannah, Jody, and
>> myself), so I'm putting it here for discussion.  The objective of this
>> change is to prevent "uncontroversial but uninteresting" PRs from falling
>> into oblivion, and try to decrease the size of the open PR stack.
>>
>> The current rule is that a PR needs positive reviews (from two
>> committers, and excluding the author if a committer) to be merged, except
>> that doc-only PRs (docstrings, rst) only needs a single positive review.
>>
>> I am proposing that, if a (non-doc) PR already has one positive review,
>> but no activity on that PR has occurred for two weeks (exact time interval
>> up to bikeshedding) [Jody suggests: and the PR has 100% code coverage],
>> then a committer (either the first reviewer, or the author if a committer)
>> can suggest that it be merged on the basis of that single review.  To do
>> so, the "sponsor" should ping all developers (@matplotlib/developers) on
>> that issue indicating that intent, and add a "single-review-merge" label on
>> the PR (so that these PRs can easily be found).  Committers are encouraged
>> to review the PR to accept and merge it or reject it or request changes on
>> it; but they can also just indicate that they consider the PR sufficiently
>> complex that a proper second review is needed before merging, or request an
>> extension, etc.  To do so they should still leave a "reject" review, even
>> if just saying "objecting to single-review merged; anyone can dismiss after
>> a second review".  However, if within another two weeks, no committer
>> voiced any objection (explicitly, i.e. by rejecting), then the PR can
>> indeed be merged on the basis of that single review.
>>
>> To avoid overwhelming the system, any committer can only "sponsor" a
>> single PR at a time.
>>
>> Thoughts?
>>
>> Antony
>> _______________________________________________
>> Matplotlib-devel mailing list
>> Matplotlib-devel at python.org
>> https://mail.python.org/mailman/listinfo/matplotlib-devel
>>
> _______________________________________________
> Matplotlib-devel mailing list
> Matplotlib-devel at python.org
> https://mail.python.org/mailman/listinfo/matplotlib-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.python.org/pipermail/matplotlib-devel/attachments/20190115/8cf55b8e/attachment.html>