[IPython-dev] Policy for closing pull requests
fperez.net at gmail.com
Mon Aug 13 02:36:41 EDT 2012
On Sun, Aug 12, 2012 at 9:53 PM, Brian Granger <ellisonbg at gmail.com> wrote:
> 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.
Sounds more than reasonable to me.
> * 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
Agreed too, on all points. When broader discussion is needed, the PR
often ends up becoming a distraction, as it's too narrowly focused on
a specific implementation.
> * What do we do about PRs that sit for a month waiting for review?
We flog ourselves with a nail-studded whip and we review them :)
Seriously, we should do our very best to avoid that from happening.
And by 'we', I mean *anyone* who is interested in the future of
IPython, not just those with commit rights! One of the best way in
which you can help our small team of core committers is by reviewing,
testing and commenting on existing PRs. It's perfectly possible for
participants to refine a PR enough that by the time a core dev has a
chance to look at it, all issues have been smoothed out and he can
just merge it right away.
So what I'd say is: if we find PRs in this kind of limbo, the right
course of action is to come to the list with a plea for help. We
shouldn't ignore contributions from anyone for that long. I would say
that, even if it means delaying one of our pet PRs that may be in the
queue, if this happens we should try really hard to prioritize those
to help move them along, *especially* if they are from new
contributors. Who knows if we'd have Thomas, Matthias, Brad or others
with us if the first time they tried to contribute it had taken months
to get the first bit of feedback.
In my mind, the priorities are:
1. PRs from new people. Reply as fast as possible, even if it means
just bouncing it back to them. It's amazing how hard people will work
for you on the internet *if you give them prompt and considered
feedback*. But if you let their contribution languish for weeks on
end, then even a simple 'change this variable name to that and we'll
merge' may get ignored, as they get disillusioned.
2. PRs from anyone else.
3. Open issues that have no attached code.
> 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
No, I don't think we need to establish such blind rules. I'm always
very skeptical of policies that try to eliminate the need for
judgment, human intervention and participation. Whether it's
zero-tolerance policies in schools sending kids into trouble because
they have a nail clipper ('a weapon') or an aspirin ('dealing
drugs'), or blind counting of journal impact factors as the sole
metric for scientific advancement, policies that remove the element of
judgment are, as far as I've seen, invariably worse than the problem
they claim to solve. So very, very strong -1 to this.
I want IPython to be a project where, if we have *one* policy, it's
that intelligent discussion comes *always* before policy.
More information about the IPython-dev