IMHO, rebasing has too high of a potential of making the history not make sense even if you don't squash, because of the way that changes can be "rebased out" of commits. So I would recommend not rebasing at all, if it's not necessary.
I definitely agree that it's a good idea to start a PR as soon as you have a single commit, so that people can start to review it and test it as you work. You can just say "work in progress" or "not ready to merge" in the OP if it needs to wait for all the commits, and then remove it when it's done. Aaron Meurer On Tue, Aug 7, 2012 at 7:18 PM, Ondřej Čertík <[email protected]> wrote: > Hi, > > Here is what I think the best workflow is when working on a GitHub > Pull Request (PR), and I would be interested if you agree with me or > not. > > 1) Develop some fix or feature locally in git and commit often [1]. > Then when I think that the branch works, I do "git rebase -i master" > and create a nice set of patches. > > 2) I create a PR > > 3) Other people comment, tests are being run, both automatically or > possibly also manually and results reported in comments. People also > pull the branch. > > 4) I address the issues again by repeating 1), but this time at the > end only rebasing code that is *not* yet in the PR, only local to my > computer. > And I *add* new patches to the public branch. In particular, I do not > rebase the branch. > > 5) Steps 3) and 4) are repeated until the PR is ready to merge. > > 6) Before it's merged, and the history is really ugly, only then it > makes sense to rebase it and create some nice set of patches. > The other circumstance is if it is some old PR and the code doesn't > merge without conflicts. Then again, rebasing is typically a cleaner > solution, unless people actually have pulled from the branch (which > usually is not the case for some old PR), in which case it's better to > "merge" with the master. > > > What makes the review quite difficult is that in the step 4) some > people simply rebase the whole thing, typically into one patch. Then > what happens is that > it becomes really difficult to figure out what exactly has changed > from my last review and comments. In particular, I left the following > comment in one PR: > > """ > First one note for any PR ---- just keep adding patches to your > branch, so that we can quickly see what has changed and one can read > the whole PR as a history or "story". If you rebase, then suddenly all > the test results stop having any sense. > Only at the very end, when the PR is ready to go in, and the history > is really ugly (which actually is rarely the case), then one can do a > quick rebase and merge it. > """ > > > What do you think? > > Ondrej > > [1] My own personal workflow is typically that I have two terminals > open, in one I periodically execute "git commit -a -m 'X'" --- so that > I don't loose my work and also so that I can see all the changes that > I made and can easily debug it if something was briefly working at > some point and stopped working later --- and run tests, in the other > terminal I have a VIM session. > > -- > You received this message because you are subscribed to the Google Groups > "sympy" group. > To post to this group, send email to [email protected]. > To unsubscribe from this group, send email to > [email protected]. > For more options, visit this group at > http://groups.google.com/group/sympy?hl=en. > -- You received this message because you are subscribed to the Google Groups "sympy" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/sympy?hl=en.
