Re: [PATCH v1 2/4] fsmonitor: update GIT_TEST_FSMONITOR support

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 14, 2018, 18:18 UTC
Message-ID: <xmqqin38aqoq.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <80988727-085f-cdd9-e5bb-a90a55e97024@gmail.com>

Ben Peart <peartben@gmail.com> writes:

> The difference here is that core.fsmonitor isn't a boolean value.  It
> is a string to a command that is executed so it can't be moved over to
> get_env_bool().

Ah, of course ;-)

Then please take the following as a review comment for 4/4; checking if each getenv(VAR) should or should not become git_env_bool() and updating them should be done as a separate change for variables whether they are being renamed or not in this series.

Show 6 quoted lines

>> I _think_ the renaming should be done without getting mixed with
>> other changes like the git_env_bool() done in 4/4.  The idea to use
>> git_env_bool() in stead of getenv() may be a good one, but then we
>> should consistently do so when appropriate, and that would make a
>> fine theme for another topic.
>>