When to close pull requests

[Anssi Kääriäinen]

I am going through pull requests which are somewhat related to ORM.

Some of the PRs are clearly not ready to be pulled in. I think the
idea is that pull requests should only be done for ready to be merged
patches, and if the patch isn't ready the pull request should be
closed. So, I am verifying if this is what we are going to do. And if
so, should we do this for any error, or just obviously wrong or
severely broken pull requests.

I am finding the pull requests as a way to submit patches less than
optimal. It leads to pull requests which do not have a backing ticket,
and having a ticket for all but the most trivial changes seems worth
it. If the commit message contains ticket reference, you can get from
a line of code to commit message to ticket details which can be very
useful. In addition, pull requests are per-person, and this can lead
to things like one user adding code, another user docs in two
different pull requests. Not good.

I think we should categorically close pull requests which are non-
trivial and do not contain ticket reference, and also those pull
requests which are more than "last polish" away from merge. Even in
the case of last polish if the author doesn't respond in reasonable
time (2 weeks for example) close the pull request.

I am worried that in addition to the large amount of open tickets we
will get a large amount of open pull requests, too.

- Anssi

[Aymeric Augustin]

Hi Anssi,

That's pretty much the conclusion we reached when we discussed how to handle PRs at DjangoCon Europe (at least, if memory serves). When you review a pull request, you should either commit it or close it.

--
Aymeric.

[Florian Apolloner]

Hi,

On Sunday, July 15, 2012 11:07:41 AM UTC+2, Anssi Kääriäinen wrote:

I think we should categorically close pull requests which are non-
trivial and do not contain ticket reference, and also those pull
requests which are more than "last polish" away from merge. Even in
the case of last polish if the author doesn't respond in reasonable
time (2 weeks for example) close the pull request.

+sys.maxint, especially on the missing ticket number…

[charettes]

FYI I wrote a small userscript[1] (greasemonkey) that linkify ticket number when browsing github.com/django/django. It's quite useful when reading the source from the web interface since tickets referenced in python comments are just a click away.

[1] https://gist.github.com/2528393