[IPython-dev] Policy for closing pull requests

Fernando Perez fperez.net at gmail.com
Mon Aug 13 23:29:49 EDT 2012


On Mon, Aug 13, 2012 at 8:14 PM, Brian Granger <ellisonbg at gmail.com> wrote:
> On Sun, Aug 12, 2012 at 11:36 PM, Fernando Perez <fperez.net at gmail.com> wrote:

>>> * 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 :)
>
> I went back and forth of this one.  I mostly agree with you.  But our
> busy schedules and the large number of PRs definitely makes it a
> challenge to review everything promptly.

I know, this one is terrible.  I really hope we never reach that
point, and that's why at least I think we can ask the wider list to
pitch in with review if we're falling behind badly.  Because if it's
only up to the core committers to keep the queue moving, we'll hit
this bad scenario way too often.

>> I want IPython to be a project where, if we have *one* policy, it's
>> that intelligent discussion comes *always* before policy.
>
> Yes, but part of my goal is that we can come up with a guidelines for
> closing PRs that are simple, and don't require long discussions.  If I

Oh, certainly: I think having guidelines like this *is* useful, as it
lets us make frequent decisions fluidly.  This is similar to how we
have other good dev guidelines that make the process easier for
everyone.  I just don't want to go too far in the direction of policy
overriding the notion that we're open to discussion *when discussion
is really necessary*.  But if the guidelines are sensible, in the vast
majority of cases they can just be applied/followed, no discussion is
necessary and they just help keep the gears in motion.

> or someone else adheres to these guidelines are you OK with PRs being
> closed without first running it by all interested parties?  To
> summarize, here are the main points:
>
> * PRs that haven't been reviewed for a month should prompt us to kick
> each other into gear.  We should ping each other, the list and
> specific people if necessary to get the review going.  The PR remains
> open and we apologize to the author for our slowness.
> * PRs that have been reviewed, but that sit for a month waiting for
> code get closed.
> * PRs that spawn larger discussions that are not resolved after a
> month get closed.
> * The person who closes an issue must open an issue that links to the

you mean 'who closes a  PR'

> PR and tell the author that we welcome them to reopen the PR when they
> start to work on it again.  If appropriate we can even give details of
> the work that needs to be done.  But the tone must be one of
> encouragement.  We can even site this guideline and our desire to keep
> PRs moving.

Yup, I think this is a *very* sensible set of guidelines, and I bet
you we'll be able to follow them more or less automatically in most
cases.

And furthermore, it means we can have a little script that runs as a
cron job somewhere and updates a little dashboard on the future wiki
showing perhaps the state of the PR queue as a green/yellow/red grid:
red ones are over one month of inactivity (hence candidates to go and
close with the above process), yellow ones have say > 2 weeks of
inactivity, and those with activity in the last two weeks are green.

I'd really like to have a birds-eye view of the PR queue like that :)
If in practice we find that it's too easy to 'refresh' a PR to green
status because of an innocent comment that doesn't really add
anything, we could tweak the above to be time since last pushed commit
instead of time since last comment...  Just an idea...

In any case, that was just me thinking about workflow tools.  But back
to the basic point of this discussion, I'm +1 on the above summary.

Thanks for taking this on!  It will help us to organize the flow better.

Cheers,

f



More information about the IPython-dev mailing list