Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes: > 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).
