All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "René Scharfe" <l.s.r@web.de>
Cc: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Philip Oakley via GitGitGadget" <gitgitgadget@gmail.com>,
	git@vger.kernel.org, "Philip Oakley" <philipoakley@iee.email>
Subject: Re: [PATCH 1/3] rebase.c: state preserve-merges has been removed
Date: Thu, 26 May 2022 13:33:51 -0700	[thread overview]
Message-ID: <xmqq5ylsxccw.fsf@gitster.g> (raw)
In-Reply-To: <19baf95d-67d4-d7ed-72a6-96d098171d3a@web.de> ("René Scharfe"'s message of "Thu, 26 May 2022 15:02:29 +0200")

René Scharfe <l.s.r@web.de> writes:

>>>  		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),
>
> Hidden options are shown if you use --help-all instead of -h.
>
> OPT_SET_INT_F always sets the struct option member "argh" to NULL.  The
> string changed above is the "help" member, not "argh".

Good points.  I do think it is OK to say REMOVED in case --help-all
asks us to show everything, even though I wonder if we can leave it
there until we remove the "support" of noticing the user asking for
a now-removed feature.

>> So there's no point in changing this string, nor to have translators
>> focus on it, it'll never be used.
>>
>> This series shouldn't fix the general issue (which parse-options.c
>> should really be BUG()-ing about, after fixing the existing
>> occurances. But For this one we could just set this to have a string of
>> "" or something, only the string you're changing in 3/3 will be seen by
>> anyone.
>
> What is the general issue?

I am afraid to ask, after having learned to be worried about those
large rearchitecting projects Ævar talks about X-<.

> 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?



  reply	other threads:[~2022-05-26 20:33 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-26  9:21 [PATCH 0/3] Die preserve ggg Philip Oakley via GitGitGadget
2022-05-26  9:21 ` [PATCH 1/3] rebase.c: state preserve-merges has been removed Philip Oakley via GitGitGadget
2022-05-26  9:40   ` Ævar Arnfjörð Bjarmason
2022-05-26 11:40     ` Philip Oakley
2022-05-26 13:02     ` René Scharfe
2022-05-26 20:33       ` Junio C Hamano [this message]
2022-05-26 21:27         ` René Scharfe
2022-05-26 23:23           ` Junio C Hamano
2022-05-27 12:35           ` Philip Oakley
2022-05-27 12:17         ` Philip Oakley
2022-05-27 15:45           ` Junio C Hamano
2022-05-27 12:12       ` Philip Oakley
2022-05-27 12:34       ` Ævar Arnfjörð Bjarmason
2022-05-26  9:21 ` [PATCH 2/3] rebase: help users when dying with `preserve-merges` Philip Oakley via GitGitGadget
2022-05-26  9:43   ` Ævar Arnfjörð Bjarmason
2022-05-26 11:44     ` Philip Oakley
2022-05-26 20:42       ` Junio C Hamano
2022-05-27 12:58         ` Philip Oakley
2022-05-27 15:54           ` Junio C Hamano
2022-05-26  9:21 ` [PATCH 3/3] rebase: note `preserve` merges may be a pull config option Philip Oakley via GitGitGadget
2022-05-26  9:50   ` Ævar Arnfjörð Bjarmason
2022-05-26 12:01     ` Philip Oakley
2022-05-26 20:55   ` Junio C Hamano
2022-05-27 12:08     ` Philip Oakley
2022-05-26  9:54 ` [PATCH 0/3] Die preserve ggg Ævar Arnfjörð Bjarmason
2022-05-26 12:57   ` Philip Oakley
2022-06-04 11:17 ` [PATCH v2 0/4] " Philip Oakley via GitGitGadget
2022-06-04 11:17   ` [PATCH v2 1/4] rebase.c: state preserve-merges has been removed Philip Oakley via GitGitGadget
2022-06-04 11:17   ` [PATCH v2 2/4] rebase: help users when dying with `preserve-merges` Philip Oakley via GitGitGadget
2022-06-04 11:17   ` [PATCH v2 3/4] rebase: note `preserve` merges may be a pull config option Philip Oakley via GitGitGadget
2022-06-06 17:57     ` Junio C Hamano
2022-06-11 14:03       ` Philip Oakley
2022-06-11 15:38         ` Philip Oakley
2022-06-11 19:22           ` Junio C Hamano
2022-06-04 11:17   ` [PATCH v2 4/4] rebase: translate a die(preserve-merges) message Philip Oakley via GitGitGadget

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqq5ylsxccw.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=l.s.r@web.de \
    --cc=philipoakley@iee.email \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.