Thread (4 messages) 4 messages, 3 authors, 2022-05-27

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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help