Denton Liu <liu.denton@xxxxxxxxx> writes: > Before, when we had the following graph, > > A---B---C (master) > \ > D (side) This is minor, but comparing the above with below > F---G topic > / > A---B---C---D---E master you'll notice that branches growing downwards in your picture (this applies also to an illustration in your tests) are off by one column. D / A---B---C \ E > @@ -1682,13 +1699,10 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) > > /* > * Check if we are already based on onto with linear history, > - * but this should be done only when upstream and onto are the same > - * and if this is not an interactive rebase. > + * but this should be done if this is not an interactive rebase. > */ Two issues. - One is shared with the original (i.e. not a problem with this patch), but what "this" refers to is not what has already been stated in the sentence. We check, to see if we are in a situation where a specific optimization is possible. But this (== optimization to fast-forward without actually replaying the commit's changes) should be done only under such and such condition. - The other is more grave. "should be done if this is not an interactive rebase" drops "only" from "only if" in the original, which changes the meaning of the sentence (it can be read as "we check if we can optimize, but the optimization should be done for rebase-i regardless of the result of that said check", which is not what you mean). Perhaps Check if we are ..., in which case we could fast-forward without replacing the commits with new commits recreated by replaying their changes. This optimization must not be done if this is an interactive rebase.