Re: [PATCH 1/3] rebase.c: state preserve-merges has been removed
From: Philip Oakley <hidden>
Date: 2022-05-27 12:42:57
Hi René On 26/05/2022 22:27, René Scharfe wrote:
Am 26.05.22 um 22:33 schrieb Junio C Hamano:quoted
René Scharfe [off-list ref] writes:quoted
quoted
quoted
OPT_SET_INT_F('p', "preserve-merges", &preserve_merges_selected, - N_("(DEPRECATED) try to recreate merges instead of " + N_("(REMOVED) try to recreate merges instead of " "ignoring them"), 1, PARSE_OPT_HIDDEN), OPT_RERERE_AUTOUPDATE(&options.allow_rerere_autoupdate),Anyway, the new help text explaining what the option once did is a bit confusing. It would be better to focus on what it's doing now (nothing) and/or why we still have it (for backward compatibility), I think.Do you mean that we should say "this option used to do such and such but it is now a no-op" after "(REMOVED)" label, instead of the above "this option does such and such"? I think "(REMOVED)" is a strong enough hint that lets us get away without saying "used to" and "but it is now a no-op", so I can accept both. Or do you mean we should say "(REMOVED) for backward compatibility, does nothing but errors out"? I would be less in faviour, then. Those who are curious enough to ask --help-all would find it more helpful if we said what it used to do. Otherwise they wouldn't be asking --help-all in the first place, no?When I see an option labeled "REMOVED" then I get confused because a thing that says it no longer exists is obviously lying
That's a misunderstanding between the response to the command line option, and the described operation of the former sub-command/option.
-- a removed option would simply not be listed. Here the feature is gone and its option remains, but only reports an educational message now.
The needed user response is more that educational. In this case (for the Series) they are in a Catch-22 situation, stuck in a no-man's land between a preserve merges that has been started, and a Git that won't proceed. Currently (prior to the series) Git will even refuse to abort..
Perhaps a better option help text would be something like "no longer supported, consider using --rebase-merges instead"?
We'll still need to say _what_ is no longer supported, to ensure the user has context. I'd agree with the suggestion aspect (Junio had commented similarly). I suspect this problem could be a long, slow burner. We so rarely remove capabilities like this, so it's tricky second guessing how users will react, or when they discover the problem. P.