Hello, On Wed, Aug 8, 2012 at 4:18 AM, Ondřej Čertík <[email protected]> wrote: > > 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. [...] > 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.
Tom and I have established a similar workflow on my pull requests, with a tiny additional detail. Once someone has started reviewing my pull request, I do not push any changes until the reviewer has finished the first round. I do modifications to my branch locally. When the reviewer announces that the first round of review is done, I push the changes and the second round of review starts. It is only very rare that I rebase-edit the pull request; I usually just add the fixes in separate commits, each of them generally addressing only a couple issues, even if this results in one-line commits. When the reviewers are done, I never mess with the history in the pull request, and it is pushed as-is. This strategy has worked nicely for me so far. I did do a couple rebases though, but that was mainly about fixing typos in commit messages. Thus I think rebasing is OK once it only introduces minor changes. Sergiu -- 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.
