On Friday, May 18, 2012 at 11:04 AM, Carl Meyer wrote: > Hi Anssi, > > Thanks for working on git usage guidelines! I very much agree that a > pull request should only be created when the contributor considers the > branch finished and ready for review and merge (for instance, there > should never be a pull request created without the necessary docs and > tests). Having lots of half-finished pull requests in the queue is a > burden on everyone. > > On 05/18/2012 05:38 AM, Anssi Kääriäinen wrote: > > The take above is that topic branches published on github are nothing > > more than a way to publish a patch set. They are _not intended for > > others to work upon_. The main problem I have with assuming the github > > branches should never be rebased is this: > > 1. A developer publishes some work on github > > 2. A reviewer spots some minor issues (typos, whitespace issues, > > pep-8 issues). > > x. > > 4. Committer merges the final work into master branch. > > > > Now, the issue is what should the original developer do in step x.? He > > could do the fixes and add a commit. However, we do not want (possibly > > multiple) extraneous "whitespace cleanup" commits in there. So, the > > branch needs to be rebased. Other option is that the committer does a > > squash merge of the work. The clearest example why we can't just merge > > the full history is that there might be commits which break Django > > totally and these commits must not be merged to master. > > > > > I think you've correctly identified the two options here: rebase or > squashed merge.* I'm not sure it's clear that rebase is the better > choice of those two. > > Advantages of rebase: > > * The contributor gets their own name on the final commit to master. > > * Less work for committers, more work for contributors (arguable whether > this is an advantage or disadvantage). > > Advantages of squashed merge: > > * Line-by-line code comments on the pull request (and in general, > in-context history of the development of the pull request) are not lost > every time the contributor rebases and force-pushes. > > * We don't ask every contributor to Django to learn advanced > history-rewriting features that some find difficult to use, and that > lead to more risk of data loss (or at least perceived data loss for > users who aren't familiar with the magic of the ref-log). Only core > committers need to use an advanced feature of git. > > All in all, I would be -1 on making rebase a required part of > contributing to Django. I would prefer to leave this choice up to the > contributor, and say something like: > > "If you want your own name on the final commit in master, feel free to > rebase your pull request down to a single commit with a well-formatted > commit message. Otherwise, we will 'merge --squash' your pull request > into a single commit on master, and credit you in the commit message." > > Carl > > > *The third option, just doing regular merges of multiple-commit pull > requests, I don't think is nearly as bad as you make it out to be: > master would never actually be in a broken state for anyone tracking it, > even if there were broken halfway states in the merged branch, and it is > possible to track the "main line" of development back through each merge > commit, without ever touching any broken or in-progress commits. But > those in-progress commits would still be in the repo, and would be shown > by default by "git log" and e.g. the github commit list - I agree that > this makes it harder to quickly scan meaningful changes in the history, > and is not preferable. > >
I personally prefer doing normal merges with --no-ff. While "clean up whitespace" commits are extraneous, they don't particularly hurt anything. If an incoming pull request is particularly messy it's easy enough to say that the pull request is sound in theory/implementation but that they need to rebase it to clean up the history. > > -- > You received this message because you are subscribed to the Google Groups > "Django developers" group. > To post to this group, send email to django-developers@googlegroups.com > (mailto:django-developers@googlegroups.com). > To unsubscribe from this group, send email to > django-developers+unsubscr...@googlegroups.com > (mailto:django-developers+unsubscr...@googlegroups.com). > For more options, visit this group at > http://groups.google.com/group/django-developers?hl=en. > > -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
