[Twisted-Python] overview: new review queue venue

Craig Rodrigues rodrigc at crodrigues.org
Sat May 21 23:52:45 MDT 2016


On Sat, May 21, 2016 at 3:12 PM, Glyph <glyph at twistedmatrix.com> wrote:

> Hooray!  We're on github now.  Next: there's the question of how to deal
> with pull requests?
>

A few people, including myself modified the text with Git instructions:
 https://twistedmatrix.com/trac/wiki/TwistedDevelopment#SubmittingaPatch

The basic idea is, "for now, if you submit a GitHub PR, you must file a
Trac ticket.  You must reference the PR in the Trac ticket,
and you must reference the Trac ticket in the PR".
If you want to change that workflow somehow, go ahead, but for starters
that should be good enough to get going.

These two pull requests followed that:

https://github.com/twisted/twisted/pull/62
https://github.com/twisted/twisted/pull/63



> Reviewing: This is the potentially slightly odd part.  I believe a review
> that doesn't result in acceptance should *close* the PR.  We need to be
> careful to always include some text that explains that closing a PR does
> not mean that the change is rejected, just that the submitter must
> re-submit.  Initially this would just mean opening a new PR, but Mark
> Williams is working on a bot to re-open pull requests when a submitter
> posts a "please review" comment: https://github.com/markrwilliams/txghbot
>

I don't agree with this.  If a PR is reviewed, and the result of the review
is "NO WAY", then I am OK with the PR being closed.
However, if a the result of the review is "needs more work before being
accepted", then
it should be possible for the submitter to add more commits to that PR,
and even "git rebase" and squash commits in that PR.

That's the process that I followed when I submitted
https://github.com/twisted/twisted/pull/62

By the way, PR 62 is the first pull request that has been successfully
submitted to Twisted, and incorporated into the code:

https://github.com/twisted/twisted/commit/95c49d74136eef420fabdeea85f650da2bc78b07

It is a rather trivial documentation fix, but I will still take credit as
the first Pull Request successful submitter! :)

--
Craig
-------------- next part --------------
An HTML attachment was scrubbed...
URL: </pipermail/twisted-python/attachments/20160521/3c170344/attachment-0002.html>


More information about the Twisted-Python mailing list