Generate rebase-equivalent merges that don't include additional new commits

[jdm]

It's a sucky experience when I push a branch for review, then add some more commits locally, rebase the branch, then push it for review again and rebase the review. Instead of seeing the old commits, the merge commit, and then the new commits, I see a merge commit which also includes the changes in the new commits, which makes it much harder to review.

[jensl]

The intended workflow, or, if you like, workaround, in such a case is that you rebase the review first, then push the new commits to the review branch.

I agree that it's a sucky experience that Critic doesn't even attempt some sort of DWIM in this case, but on the other hand, in the general case, it's not necessarily straight-forward to determine what is the old changes and what is new commits, given that it's expected that a rebase may also involve various degree of rewritten history.

[jdm]

My experience has been that contributors to our Critic-using project never rewrite previous commit messages while reviews are ongoing (not least because we discourage rewriting history). That means that it should be possible to determine new commits just based on the commit messages alone, and it's frustrating that this isn't seen as a viable option.

[jensl]

My experience is that I rewrite previous commit messages quite often. I usually don't mix history rewriting and rebasing onto a different (typically newer) upstream commit in the same rebase, though.

My experience from the environment where Critic was primarily developed (i.e. at Opera Software) is that developers do all sorts of things, and sometimes not at all what I'd expect them to do. :-)

So, I'd say that Critic has been designed to not make assumptions about the workflow people use "outside of Critic", and rather to allow as efficient use within Critic regardless of how that workflow is. (This is not meant to say that I've succeeded to any particular degree in this; I'm simply talking about what I've aimed for, and not aimed for.)

A problem with the type of heuristic that you suggest is that it can fail rather badly if the workflow wasn't the one you're describing. So it would have to be coupled, I think, with some degree of automatic sanity checking, falling back to something else, or some way of previewing Critic's representation of a rebase and adjustment of Critic's attempt at figuring out what happened. And that's of course quite a bit more work than it would be to simply implement the suggested heuristic in the first place.