Re: [PATCH] rebase: remove the rebase.useBuiltin setting
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Feb 13, 2019, 20:46 UTC
- Message-ID
- <xmqqva1nbeno.fsf@gitster-ct.c.googlers.com>
- In-Reply-To
- <20190213142209.8226-1-avarab@gmail.com>
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
Show 17 quoted lines
> 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. > ... >> This patch breaks the test suite (with these two new tests) under >> GIT_TEST_REBASE_USE_BUILTIN=false. So a 2.21.0-rc0 regression. >> >> It would have been better to raise this before the rc period, but I just >> noticed this now, but we can now: >> >> 1. Skip the test under that mode >> 2. Fix the shell code to do the same thing >> 3. Just remove the shell code & GIT_TEST_REBASE_USE_BUILTIN=false mode >> >> Maybe we should just do #3. The reason for the escape hatch was in case >> we had bugs, and now we've had a full release cycle, but maybe that's >> too early...
If a new feature is added to the built-in version, I do not think it is a good use of our time to backport it to the scripted version, if only to make the USE_BUILTIN=false test pass, especially when the problematic thing is a fringe feature, lack of which would not affect the real users too much.
So I do agree that #2 is a bad choice.
However, it is way too late in the cycle to say that we will ship without the escape hatch for the upcoming release, so #3 is a non starter. And you are reading too much into a full release cycle, which is merely less than 1k commits and a bit short of 3 months.
It would however be long enough to declare victory _immediately after_ the upcoming release and start the next cycle without the escape hatch. At that point we'd be committed to maintain only the built-in version.
The more important every-day features should still be covered by tests, if the scripted ones are to be kept as escape hatch. So to me #1 looks like the only sensible choice at this point, if you want to see a test cycle with USE_BUILTIN=false to fully pass (i.e. by skipping the ones that are known not to pass).