From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Fri, 15 Mar 2019 16:11:40 GMT
Subject: Re: [PATCH v3] rebase: remove the rebase.useBuiltin setting
Message-ID: <87wol09kxv.fsf@evledraar.gmail.com>
In-Reply-To: <nycvar.QRO.7.76.6.1903151643300.41@tvgsbejvaqbjf.bet>


On Fri, Mar 15 2019, Johannes Schindelin wrote:

> Hi Ævar,
>
> On Fri, 15 Mar 2019, Ævar Arnfjörð Bjarmason wrote:
>
>> Remove the rebase.useBuiltin setting, which was added as an escape
>> hatch to disable the builtin version of rebase first released with Git
>> 2.20.
>>
>> See [1] for the initial implementation of rebase.useBuiltin, and [2]
>> and [3] for the documentation and corresponding
>> GIT_TEST_REBASE_USE_BUILTIN option.
>>
>> Carrying the legacy version is a maintenance burden as seen in
>> 7e097e27d3 ("legacy-rebase: backport -C<n> and --whitespace=<option>
>> checks", 2018-11-20) and 9aea5e9286 ("rebase: fix regression in
>> rebase.useBuiltin=false test mode", 2019-02-13). Since the built-in
>> version has been shown to be stable enough let's remove the legacy
>> version.
>>
>> As noted in [3] having use_builtin_rebase() shell out to get its
>> config doesn't make any sense anymore, that was done for the purposes
>> of spawning the legacy rebase without having modified any global
>> state. Let's instead handle this case in rebase_config().
>>
>> There's still a bunch of references to git-legacy-rebase in po/*.po,
>> but those will be dealt with in time by the i18n effort.
>>
>> Even though this configuration variable only existed for one release
>> let's not entirely delete the entry from the docs, but note its
>> absence. Individual versions of git tend to be around for a while due
>> to distro packaging timelines, so e.g. if we're "lucky" a given
>> version like 2.21 might be installed on say OSX for half a decade.
>>
>> That'll mean some people probably setting this in config, and then
>> when they later wonder if it's needed they can Google search the
>> config option name or check it in git-config. It also allows us to
>> refer to the docs from the warning for details.
>>
>> 1. 55071ea248 ("rebase: start implementing it as a builtin",
>>    2018-08-07)
>> 2. d8d0a546f0 ("rebase doc: document rebase.useBuiltin", 2018-11-14)
>> 3. 62c23938fa ("tests: add a special setup where rebase.useBuiltin is
>>    off", 2018-11-14)
>> 3. https://public-inbox.org/git/nycvar.QRO.7.76.6.1903141544110.41@tvgsbejvaqbjf.bet/
>> ---
>
> With the obviously intended SOB line: ACK!

Urgh, Junio: If there's no other comments/concerns on v3 requiring a v4
this has my:

    Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>