[IPython-dev] Policy for closing pull requests

Brian Granger ellisonbg at gmail.com
Mon Aug 13 00:53:35 EDT 2012


On Sun, Aug 12, 2012 at 9:33 PM, Fernando Perez <fperez.net at gmail.com> wrote:
> On Sat, Aug 11, 2012 at 7:17 PM, Aaron Meurer <asmeurer at gmail.com> wrote:
>> One other important rule that we have in SymPy is that if we close a
>> pull request that still needs work (as opposed to an outright
>> rejection), we make sure that it is mentioned in an open issue.
>> Otherwise, it will be forgotten forever.
>
> With Aaron's additional point, I'm +1 on the idea, whereas the
> original proposal seemed too aggressive to me.  I understand the need
> to optimize our limited resources so we can focus on code that has a
> hope of getting merged, but as originally presented I think that it
> pushed too hard in that direction and had the danger of alienating
> contributors (we don't want to 'solve' our current problem of having
> too many open PRs simply by losing contributors, that would be
> throwing out the baby with the bathwater).
>
> So I'm +1 on the original proposal, but once amended with:
>
> - Every time we close a PR because it has become 'dormant' (but one we
> assume would get merged with some extra work), we do two things:
>
> 1. In the closing comment, explain clearly to the author this, so they
> realize the closing isn't a rejection, and that the simple act of
> pushing a few commits would be sufficient to get it reopened (and
> possibly merged if that completes what was missing).
>
> 2. We create a regular issue linking to this PR, with a label
> 'dormant-PR'.  This would let us quickly use the issue sorter to have
> a look at dormant PRs, which are also candidates for sprints, when a
> core developer wants to do some quick housecleaning, etc.

I agree that Aaron's idea is important and I am +1 on implementing
this as you describe.  I just want their to be agreement on the
criteria we use to decide *when* we take such an action.  How do you
feel about the following:

* When a PR has been reviewed and is waiting additional code and sits
in this state untouched for 1 month.
* When the review process can't seem to reach a solid conclusion
because there are larger issues or technical details that have to be
worked out.  Once it reaches this point and sits for a month, we
should close the PR and open an issue to continue the larger
discussion.  I suppose if it becomes obvious that it is in this state
in a shorter amount of time, we could close it earlier.  The goal is
to have PRs reflect code that is actively moving forward towards a
merge.
* What do we do about PRs that sit for a month waiting for review?

We could go with a much simpler approach and say "regardless of the
reason if a PR hasn't been touched for a month, we close it and open
and issue."  A more uniform criteria might come across as less
personal.

Thoughts?

Cheers,

Brian


>
> How does that sound?
>
> f
> _______________________________________________
> IPython-dev mailing list
> IPython-dev at scipy.org
> http://mail.scipy.org/mailman/listinfo/ipython-dev



-- 
Brian E. Granger
Cal Poly State University, San Luis Obispo
bgranger at calpoly.edu and ellisonbg at gmail.com



More information about the IPython-dev mailing list