[Twisted-Python] github, again

Terry Jones terry at jon.es
Tue Jun 4 12:10:19 MDT 2013


The general workflow that's being described is:

 - You open an issue for all bugs, enhancements, etc.
 - When someone starts working on one of these, they create a branch (we
use descriptive branch names and put -NNNN at the end, with the issue
number).
 - When the branch reaches the point where the author feels it could be
merged or wants to open it up for easy discussion, they send a pull request.

The pull request is a signal to the people working on the base code (that
the pull request is against/for, let's just call it "master") that the
author of the branch has it in mind to merge their branch into master. That
may be a way off, it may never happen, or it could be done almost
immediately. Github doesn't have any formal mechanism for having a branch
approved, or even a mechanism for saying "I'm interested in this pull
request and I intend to review / comment on it before it's merged, so
please don't merge it until I've +1'd".  That's a weakness and a strength.
We adopted the convention (as @jkakar mentioned) of having interested
parties just edit the pull request description, to put in a string like
"terrycojones: **pending**" to indicate that you're planning to comment.

The pull requests are discussions about the code, and the code changes
while the pul request is outstanding. That's normal. Other people, who have
the right permission, might want to push changes into the branch that's
being worked on. E.g., there could be a jkakar/fix-race-condition-4772
branch. I switch onto that branch (git remote update --prune jkakar;  git
co jkakar/fix-race-condition-4772) and run the tests, take a look, etc.  I
decided to help out by making a few changes, so I make my own branch:  git
co -b fix-race-condition-4772.  That creates
terrycojones/fix-race-condition-4772 which I can work on, commit to, and
push back to github.  I could then send a pull request to merge that branch
into jkakar/fix-race-condition-4772 or if I have the right perms, just push
it directly into that branch.

The diff in the pull request always reflects the latest changes, as you'd
expect. So over time as there is discussion & code change around the pull
request, the diff will change. As I mentioned, parts that are fixed will
have the conversation disappear (this sounds alarming, perhaps, but it
works).

At some point everyone who's interested will have contributed to the
discussion, to the code, and signed off.  Then you merge it, using the web
UI.  Sometimes a pull request doesn't reach the point where there's
agreement that it should be merged. When that happens, we usually close the
pull request so it's not cluttering up the pull requests page. The branch
doesn't go away, of course, and can be re-proposed for merging via a later
pull request.

I hope that helps.  I think most of this is open-ended and teams can choose
the conventions that suit them. The above is just what we've done, but it
does seem to match up pretty closely with some of the links people have
been posting.

One point of difference that I don't know the best answer to: We tend to
each have our own fork of  a repo, and to send pull requests into the repo
"owned" by the organization. Others (including Github themselves) just have
one repo and anyone can make a branch in that repo and propose it for
merging into the master of the same repo.  I think I prefer the former,
though it has a little more overhead and it requires people to do a git
remote add git at github.com:name/project.git for the other people whose
changes you want to track and pull in etc (via git remote update --prune).

Terry




On Tue, Jun 4, 2013 at 5:42 PM, Tobias Oberstein <
tobias.oberstein at tavendo.de> wrote:

> in general: +1 for this
>
> > Finally, my own minor concern: Github has no notion of a "code review"
> as a unit of work.  A pull request is just "open" until it is "closed".
> > <snip>
>
> I _think_ the following is true (if so, I find that strange) - pls correct
> me if I'm wrong:
>
> A pull request is not tied to a specific rev, but only to a source
> repo/branch.
> While the pull isn't merged (and hence closed), more commits on the source
> repo/branch can be added.
>
> So a pull request is kind of moving target ..
>
> GitHub seems to view that as a feature, not bug:
> """
> Pull requests can be sent from any branch or commit but it's recommended
> that a topic branch be used so that follow-up commits can be pushed to
> update the pull request if necessary.
> """
> https://help.github.com/articles/using-pull-requests
>
> """
> It's important to use a new branch for pull requests for several reasons:
>
> It allows you to submit multiple pull requests without confusion. The
> classical Github gotcha is to continue committing to a pull request branch
> after making the initial request. When these commits are pushed to the
> remote, they will become part of the original pull request which often ends
> up conflating unrelated functionality.
> """
>
> http://codeinthehole.com/writing/pull-requests-and-other-good-practices-for-teams-using-github/
>
> However: https://github.com/blog/712-pull-requests-2-0
> I'm not sure how to interpret that ..
>
> /Tobias
>
> _______________________________________________
> Twisted-Python mailing list
> Twisted-Python at twistedmatrix.com
> http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: </pipermail/twisted-python/attachments/20130604/f744e4ad/attachment-0002.html>


More information about the Twisted-Python mailing list