On Fri, Apr 10, 2020 at 6:11 PM Jonathan Tan <jonathantanmy@xxxxxxxxxx> wrote: > > > > Reapply all clean cherry-picks of any upstream commit instead of > > > dropping them. (If these commits then become empty after rebasing, > > > because they contain a subset of already upstream changes, the > > > behavior towards them is controlled by the `--empty` flag.) > > > > Perhaps add "preemptively" in there, so that it reads "...instead of > > preemptively dropping them..."? > > Sounds good. Yes I can do this. > > > > If this works, I'll send out a new version containing Elijah's patches > > > and mine in whatever branch my patch shows up in [1]. > > > > > > [1] https://lore.kernel.org/git/xmqqd08fhvx5.fsf@xxxxxxxxxxxxxxxxxxxxxx/ > > > > Yeah, I was making changes to this exact same area in my series to > > reference your flags.[2] > > > > [2] https://lore.kernel.org/git/e15c599c874956f1a297424c68fe28e04c71807b.1586541094.git.gitgitgadget@xxxxxxxxx/ > > > > Would you mind if I took your proposed changes, put them in your > > patch, and then rebased your patch on top of my series and touched up > > the wording in the manpage to have the options reference each other? > > Go ahead! Thanks. Cool, please double check that I made the changes as you expected: https://lore.kernel.org/git/20d3a50f5a4bf91223c1b849d91e790683d70d66.1586573068.git.gitgitgadget@xxxxxxxxx/ > > > > Why not just list --keep-cherry-pick[s] in the list of options that > > > > require use of the merge backend (i.e. the list containing '--merge') > > > > instead of adding another sentence here? > > > > > > My reading of the list containing "--merge" is that they *trigger* the > > > merge backend, not require the merge backend. My new option requires but > > > does not trigger it (unless we want to change it to do so, which I'm > > > fine with). > > > > Interesting; what part of the man page comes across that way? That > > may just be poor wording. > > "--merge" is documented as "Use merging strategies to rebase", which I > interpret as triggering the merge backend. There are other things in the > list like "--strategy" and "--interactive", which seem to be things that > trigger the merge backend too, so I concluded that the list is about > triggering the merge backend, not requiring it. > > > However, if an option requires a certain backend, is there a reason > > why we would want to require the user to manually specify that backend > > for their chosen option to work? We know exactly which backend they > > need, so we could just trigger it. For every other case in rebase I > > can think of, whenever a certain backend was required for an option we > > always made the option trigger that backend (or throw an error if a > > different backend had already been requested). > > I guess I wanted to leave open the option to have the same feature in > the "apply" (formerly "am") backend. The use cases I am thinking of > won't need that in the near future (for partial clone to make use of it > in the "apply" backend, the "apply" backend would have to be further > improved to batch fetching of missing blobs), though, so it might be > best to just require and trigger "merge" (like the other cases you > mention). I'll do that in the next version. Putting them in the list doesn't mean that they're designed to only work with one backend, just a reflection of what the current requirements/incompatibilities are. We've removed things from the list before when we implemented it in the other backend(s).
