[IPython-dev] Policy for closing pull requests
Aaron Meurer
asmeurer at gmail.com
Mon Aug 13 02:59:46 EDT 2012
On Mon, Aug 13, 2012 at 12:36 AM, Fernando Perez <fperez.net at gmail.com> wrote:
> 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
>> merge.
>
> 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
>> personal.
>
> 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.
Amen to this! Eric Jones expressed a similar sentiment in his SciPy
2011 keynote about unnecessary rules and their unintended
consequences. So "a month" should be, at best, a rule of thumb, and
then, really only if it's necessary beyond the base rule of thumb,
"use common sense".
Aaron Meurer
>
> Cheers,
>
> f
> _______________________________________________
> IPython-dev mailing list
> IPython-dev at scipy.org
> http://mail.scipy.org/mailman/listinfo/ipython-dev
More information about the IPython-dev
mailing list