On Sun, Jul 12, 2015 at 2:53 AM, Joachim Durchholz <[email protected]> wrote: > Am 12.07.2015 um 00:17 schrieb Jason Moore: >> >> FYI: We had a discussion at SymPy on how to synchronize with master and >> decided that we no longer want to allow or encourage rebasing after a pull >> request is made to the main SymPy repo. > > > I think that's overstated. > > I see some reasons against merging: > > A) A history with merges is harder to read. You need to track multiple > parallel developments, and that's particularly hard if merge conflicts were > resolved (because some of the code you see didn't get into master, at least > not in the form it's committed initially). > > B) The results from `git bisect` become harder to read if a bug stems from > the combination of two changes that were made in parallel branches: the > bisect will point at the merge commit, but the actual problem happened in > some earlier commit. > The more parallel branches in existence, the more prominent this will > become.
I agree with this --- many times there is just a few lines change, that took 50 commits to get done, because the author was learning how to make it work. For these cases I recommend to send a new PR with a polished (=rebased) history. Then there is no problem. Ondrej > >> The main reason is that we stall too many new contributors by forcing them >> to do too much git kung fu. > > > This problem can be mitigated via interactive rebasing, but I agree it's an > extra hoop to jump through and cannot be asked of everyone. > >> But there are additional important reasons too: >> >> >> - Once a pull request is "public" the history should not change because >> other people may have pull it. > > > Does anybody ever pull from a pull request? > I can see that happening when collaborating on a fix, but it's not something > affecting newbies. > >> - Rebasing with merge conflicts is much more difficult > > > Only if you try to rebase everything in a single go. You may end up > resolving the same conflict over and over for each commit, and that can get > confusing very quickly, and it's also annoying and a lot of needless work. > > However, there is a process that's actually quite easy: > 1) `git rebase --interactive` first; this is a rebase *within your working > branch*. > I also squash commits that were a continuation of previous work. I.e. if I > have a sequence of 1.code - 2.reformat - 3.code - 4.bugfix - 5.reformat - > 6.code - 7.bugfix - 8.reformat, I like to rearrange this to 1.code - 3.code > - 4. bugfix - 6.code - 7.bugfix - 5.reformat - 7.reformat, and then I squash > this into two commits, one with 13467 for the coding and 57 for > reformatting. > If this still is confusing, this can be broken down into a series of > primitive steps: (a) swap two commits, leaving all others alone, (b) merge > two adjacent commits, leaving all others alone; repeat until the history is > rewritten in a way that's easy to read and review. > 2) `git fetch master && git rebase master`. This will expose the merge > conflicts, but now it's far less commits, and it's also giving you much less > pain because most conflicts will apply just in one commit. > >> - Reviewers are no longer able to see commit by commit changes in a review >> process. > > > Only if you squash everything into a single commit. > Reordering and squashing commits makes sense if some work jumped between > multiple things, such as reformatting, fixing docstrings, and actual code > changes (and if the code changes touched multiple loosely-related points, it > may make sense to keep these in separate commits anyway). > >> - Github does not manage the diffs in a PR review if a rebase happens. > > > You mean the "click here to see outdated diffs" thing? > Sure, part of the discussion are removed one more click, which means that > people won't know to look there and usually won't find them. > However, if a discussion is relevant for posteriority, it should have been > merged into the commit in some way, either into the commit comment or in a > code comment or docstring. After all, people who're looking at `git > annotate` or `git log` aren't going to see these discussions either. > >> If you want to rebase then you should do it before you make a pull >> request, >> either locally or on your fork's branch. > > > Oh, now I see what's the concern - indeed, rebasing on the main repo is bad, > very bad, and a real no-go. But that's a non-issue! > > For pull requests, they're on separate branches, and people don't work on > the PR branch of anybody else (unless they explicitly have agreed to > cooperate in that way, in which case the work branch should indeed not be > rebased anymore unless all downstream collaborators agree). > > Newbies don't even have the permissions to modify the main repo. > > The one point where I see strong language against rebasing justified is when > merging into master on the main repo. This is merge, and merge only, and > should be written in bold letters into the project maintenance guidelines. > For people who contribute pull requests, I do not think that applies. > > YMMV, but I'd be interested to hear why. > > Regards, > Jo > > -- > You received this message because you are subscribed to the Google Groups > "sympy" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected]. > To post to this group, send email to [email protected]. > Visit this group at http://groups.google.com/group/sympy. > To view this discussion on the web visit > https://groups.google.com/d/msgid/sympy/55A22B0B.20405%40durchholz.org. > > For more options, visit https://groups.google.com/d/optout. -- You received this message because you are subscribed to the Google Groups "sympy" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. Visit this group at http://groups.google.com/group/sympy. To view this discussion on the web visit https://groups.google.com/d/msgid/sympy/CADDwiVAMf%2BPnH2vu_Kk4q_-LqrQXG%3DnEX91PrQhidqbtSxJN4A%40mail.gmail.com. For more options, visit https://groups.google.com/d/optout.
