Joachim, Thanks for the start of the bike shedding, you are becoming a master at it.
You can always do what you want on your own pull request if your reviewers are fine with your synchronization strategy, but we want to make the default behavior be centered on merging. This means that reviewers should not ask for or encourage rebasing on an open pull request and thus we should encourage merging. It's not that big of deal and we do not see any negative aspects from disallowing rebasing that outweighs the benefits it gains. If you actually have very specific proof that the cost from this guideline outweighs the benefits then we can discuss. But this has been SymPy's underlying guideline for sometime now. Rebasing has crept back into people's minds lately for some reason, so this simply clarifies an existing guideline which many of the main devs reached consensus on at the SciPy BoF session. Jason moorepants.info +01 530-601-9791 On Sun, Jul 12, 2015 at 3: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. > > 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/CAP7f1AioDMFpBGmbkxM%3D2wwbt0Ze1jv9J%3DtoinoDzq%3DBNSznug%40mail.gmail.com. For more options, visit https://groups.google.com/d/optout.
