> -----Original Message----- > From: Ben Peart [mailto:peartben@xxxxxxxxx] > Sent: Monday, September 18, 2017 9:07 AM > To: David Turner <David.Turner@xxxxxxxxxxxx>; 'Ben Peart' > <benpeart@xxxxxxxxxxxxx> > Cc: avarab@xxxxxxxxx; christian.couder@xxxxxxxxx; git@xxxxxxxxxxxxxxx; > gitster@xxxxxxxxx; johannes.schindelin@xxxxxx; pclouds@xxxxxxxxx; > peff@xxxxxxxx > Subject: Re: [PATCH v6 04/12] fsmonitor: teach git to optionally utilize a file > system monitor to speed up detecting new or changed files. > > Thanks for taking the time to review/provide feedback! > > On 9/15/2017 5:35 PM, David Turner wrote: > >> -----Original Message----- > >> From: Ben Peart [mailto:benpeart@xxxxxxxxxxxxx] > >> Sent: Friday, September 15, 2017 3:21 PM > >> To: benpeart@xxxxxxxxxxxxx > >> Cc: David Turner <David.Turner@xxxxxxxxxxxx>; avarab@xxxxxxxxx; > >> christian.couder@xxxxxxxxx; git@xxxxxxxxxxxxxxx; gitster@xxxxxxxxx; > >> johannes.schindelin@xxxxxx; pclouds@xxxxxxxxx; peff@xxxxxxxx > >> Subject: [PATCH v6 04/12] fsmonitor: teach git to optionally utilize > >> a file system monitor to speed up detecting new or changed files. > > > >> +int git_config_get_fsmonitor(void) > >> +{ > >> + if (git_config_get_pathname("core.fsmonitor", &core_fsmonitor)) > >> + core_fsmonitor = getenv("GIT_FSMONITOR_TEST"); > >> + > >> + if (core_fsmonitor && !*core_fsmonitor) > >> + core_fsmonitor = NULL; > >> + > >> + if (core_fsmonitor) > >> + return 1; > >> + > >> + return 0; > >> +} > > > > This functions return values are backwards relative to the rest of the > git_config_* functions. > > I'm confused. If core.fsmonitor is configured, it returns 1. If it is not > configured, it returns 0. I don't make use of the -1 /* default value */ option > as I didn't see any use/value in this case. What is backwards? The other git_config_* functions return 1 for error and 0 for success. > > [snip] > > > > +> /* > > +> * With fsmonitor, we can trust the untracked cache's valid field. > > +> */ > > > > Did you intend to make a comment here? Sorry. I was going to make a comment that I didn't see how that could work since we weren't touching the untracked cache here, but then I saw the bit further down. I'm still not sure it works (see comment on 10/12), but at least it could in theory work.