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

[Ben Peart <peartben@gmail.com>]

On 9/14/2018 1:15 PM, Junio C Hamano wrote:
> Ben Peart <benpeart@microsoft.com> writes:
> 
>> diff --git a/config.c b/config.c
>> index 3461993f0a..3555c63f28 100644
>> --- a/config.c
>> +++ b/config.c
>> @@ -2278,7 +2278,7 @@ int git_config_get_max_percent_split_change(void)
>>   int git_config_get_fsmonitor(void)
>>   {
>>   	if (git_config_get_pathname("core.fsmonitor", &core_fsmonitor))
>> -		core_fsmonitor = getenv("GIT_FSMONITOR_TEST");
>> +		core_fsmonitor = getenv("GIT_TEST_FSMONITOR");
> 
> Sorry for not noticing earlier, but unlike 4/4 that changed
> getenv(VAR) to git_env_bool(VAR, 0) "while at it", this leaves it to
> getenv(VAR), meaning "if it is set to any non-empty string, it is
> true".  Is there a reason for this discrepancy?
> 

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().

> 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.
>