> On May 21, 2016, at 10:52 PM, Craig Rodrigues <rodr...@crodrigues.org> wrote:
> 
> On Sat, May 21, 2016 at 3:12 PM, Glyph <gl...@twistedmatrix.com 
> <mailto:gl...@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 
> <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/62>
> https://github.com/twisted/twisted/pull/63 
> <https://github.com/twisted/twisted/pull/63>

Sorry, I guess I wasn't clear.  I know that PRs are presently a potential 
alternative to a diff, and that we are still using Trac for ticketing.  I want 
to make it possible to avoid using Trac for ticketing; perhaps switching to 
github issues entirely.  Right now, PRs are still ignored; the thing reviewers 
are paying attention to is the review queue in Trac, and that is sub-optimal, 
since it requires new contributors to do something unfamiliar.

> 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 
> <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.

I understand that this is your feeling, but do you have any reasoning as to why 
you believe that this should be the case?  "I don't feel like it" isn't selling 
me.

> 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,

Technically speaking, "PRs" are not things that you add commits to.  You add 
commits to branches, and the PR points to a branch.  Closing your PR will not - 
can not, if it's on your own fork - delete your branch.

> and even "git rebase" and squash commits in that PR.

Please do not use squash commits.  See http://mjg59.dreamwidth.org/42759.html.

> That's the process that I followed when I submitted 
> https://github.com/twisted/twisted/pull/62 
> <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
>  
> <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! :)

First is first, trivial or not :-).

-glyph


_______________________________________________
Twisted-Python mailing list
Twisted-Python@twistedmatrix.com
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python

Reply via email to