Subject: Re: [PATCH v3 6/6] core.fsyncobjectfiles: enable batch mode for testing

From: Junio C Hamano <[email-masked]>
Date: Wed, 15 Sep 2021 16:12:08 -0700

Neeraj Singh <[email-masked]> writes:

> This commit is mainly to ensure that we get coverage of batch mode on all
> platforms in the CI infrastructure.  I don't believe it should be included in
> mainline git without significantly more discussion and experimentation.

Am I incorrect to say that only just a handful of code paths can
take advantage of the bulk checkin "plugging-unplugging" feature to
begin with, so running _all_ the existing tests that cover
everything with this core.fsyncobjectfiles=batch setting is rather
pointless?

If so, perhaps instead of 6/6, you should identify key code paths
that would be affected by this feature (perhaps "git add" is one of
them), and either write a new test script dedicated for this feature
or piggy-back on existing test scripts that already tests the code
paths and adding new test pieces there that exercise this new feature.

If it is a good idea to run all the tests with core.fsyncobjectfiles
set to batch, however, it probalby is easiest to invent a new
environment variable GIT_TEST_FORCE_CORE_FSYNCOBJECTFILES and have
it honored as the default when it is set, and add a NEW CI job that
exports the environment with the value "batch".  Other people
(including the ones from Microsoft, I think) are much more familiar
than I am on how to make this kind of thing work in GitHub Actions.

> Do you think that feature.experimental is a good place to put this right away,

I think feature.experimental should be used for something that we
hope would benefit "everybody", not "most of the users".  This is a
promise to our testers, who opt into "early preview" of upcoming
features should not be subjected to "this may or may not give better
experiences depending on your workflow".  They may already be
enjoying and even relying on other experimental features by opting
in, and we should strive not to add a reason for them to turn the
feature.experimental bit off by saying "this new experimental feature
that recently joined does not work for my use case."