Elijah Newren <newren@xxxxxxxxx> writes: >> +--keep-cherry-pick:: >> +--no-keep-cherry-pick:: > ... > I'm slightly worried that "keep" is setting up an incorrect > expectation for users; in most cases, a reapplied cherry-pick will > result in the merge machinery applying no new changes (they were > already applied) and then rebase's default of dropping commits which > become empty will kick in and drop the commit. Yes. > Maybe the name is fine and we just need to be more clear in the text > on the expected behavior and advantages and disadvantages of this > option: > > If `--keep-cherry-picks` is given, all commits (including these) will be > re-applied. Note that cherry picks are likely to result in no changes > when being reapplied and thus are likely to be dropped anyway (assuming > the default --empty=drop behavior). The advantage of this option, is it > allows rebase to forgo reading all upstream commits, potentially > improving performance. The disadvantage of this option is that in some > cases, the code has drifted such that reapplying a cherry-pick is not > detectable as a no-op, and instead results in conflicts for the user to > manually resolve (usually via `git rebase --skip`). True. So instead of letting the machine match commits on the both sides up, the end-user who is rebasing will find matches (or near matches) and manually handle them. It would be a good idea to describe the pros and cons for the option (which I think has already been written fairly clearly in the proposed patch). > It may also be helpful to prevent users from making a false inference > by renaming these options to --[no-]reapply-cherry-pick[s]. Hmm, yeah, that may not be a bad name.