Hi, On Fri, Oct 19, 2018 at 1:46 AM Ryan Blue <rb...@netflix.com.invalid> wrote:
> I find that it’s easier to rebase because it requires fixing individual > commits that are smaller. Our personal experience with this feature branch was that the codebases really diverged and conflict resolution with rebasing was not feasible. Some of the upstream changes required modifications that we could add as a new commit with merging, but would have had to introduce retroactively from the very first commit with rebasing. >> Rebasing leads to rewriting history, which goes against the every change is >> reviewed on the feature branch approach. > > Again, reviews should validate tests that can be used here. The squashing > approach that we’ve used for a long time relies on the same principle. Squashing changes history as well indeed, but this only affects the number and message of commits. The code itself does not change. With a rebase, code also changes and in such a way that we have no way of differentiating what has already been reviewed and what is new. I don't think that passing unit tests are a good enough substitute for reviewing the conflict resolution, especially when it involves extensive changes, as it required in our case. > Rebasing the feature branch leads to loss of review comments on existing > and merged pull requests, since those are associated with specific git > hashes. > > Comments should be attached to PRs, not hashes. Github associates each review comment with a specific line in a specific commit. When someone force pushes a rebase onto their branch that serves as the basis of the pull request, the earlier comments are left without context as the code changes they were attached to are no longer available. Br, Zoltan
