On Wed, Jul 15, 2015 at 12:46 PM, Jason Moore <moorepa...@gmail.com> wrote: > Yes I agree, if your history is so heinous, and bothers you so much, and you > really really want to rebase, just make a new PR for pure merging after the > review is done (or discuss with you reviewers about it). But otherwise it > doesn't really matter. For example, if git didn't have a rebase command, > then everything would work just fine and we would not even care about this > issue. Git works perfectly fine without the rebase command, it is a complete > functioning and useful tool without it. Cleaning the history is 99% of the > time an unnecessary step. The arguments for cleaning the history are very > superficial and really amount to "I don't like that the history is messy.". > There is very little gain from rebasing but the overhead is high as we've > shown. The rebase command does not actually solve any problems, it just > introduces them. The main goal here is that reviewers should not recommend > rebasing so I'm not fond of Matthew's suggestion as it is written because it > removes that guideline. We've also said that you can rebase your own PR if > you discuss that with your reviewers. So any rebasing purists can work that > out on their PRs.
Yes, once the PR is up and people start reviewing, and I start following the author's branch and have it checked out (and possibly have some commits of my own in there), then a rebase is a tremendously impolite thing to do, as it screws my own git repo, and it also screws the PR itself, as the old discussion is suddenly hanging there without commits and it is not clear what changes were made. So for these reasons, we should say no rebase after you submit a PR for review. That being said, once you become experienced developer, you should take pride in submitting a nice set of polished patches, with good git commit logs, each patch logically self consistent, so that it is a pleasure to look at your work, and easy to review for reviewers. And for that you need rebase. There is high value in having good commit history, for debugging or just learning what was done. So I agree with Jo on this. But the point is that you do this before you post a PR. >From that point on, you just keep committing and merging. And for cases when the history really gets messy, you just send a new PR with polished commits. This happens for new authors, but sometimes I do that myself. I see no harm in submitting a new polished PR and closing the old one. So this should be our recommendation. Jason already agreed to the above. Yo and others, do you also agree with the above? Ondrej -- 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 sympy+unsubscr...@googlegroups.com. To post to this group, send email to sympy@googlegroups.com. 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/CADDwiVB2GdY_Ay6272a7vYMFPb9OY20T9PNbis7%2BTsEtfYsJ2w%40mail.gmail.com. For more options, visit https://groups.google.com/d/optout.
