On Sat, May 21, 2016 at 3:12 PM, Glyph <glyph@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/95c49d74136eef420fabdeea85f650da2b... It is a rather trivial documentation fix, but I will still take credit as the first Pull Request successful submitter! :) -- Craig