From: Ben Peart Date: Fri, 14 Sep 2018 18:05:46 GMT Subject: Re: [PATCH v1 2/4] fsmonitor: update GIT_TEST_FSMONITOR support Message-ID: <80988727-085f-cdd9-e5bb-a90a55e97024@gmail.com> In-Reply-To: On 9/14/2018 1:15 PM, Junio C Hamano wrote: > Ben Peart 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. >