Re: [PATCH 06/12] fsmonitor: clarify handling of directory events in callback

> "Jeff Hostetler via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> From: Jeff Hostetler <jeffhostetler@github.com>
>>
>> Signed-off-by: Jeff Hostetler <jeffhostetler@github.com>
>> ---
>>   fsmonitor.c | 47 +++++++++++++++++++++++++++++++++--------------
>>   1 file changed, 33 insertions(+), 14 deletions(-)
>>
>> diff --git a/fsmonitor.c b/fsmonitor.c
>> index 614270fa5e8..754fe20cfd0 100644
>> --- a/fsmonitor.c
>> +++ b/fsmonitor.c
>> @@ -219,24 +219,40 @@ static void fsmonitor_refresh_callback_unqualified(
>>   	}
>>   }
>>   
>> -static void fsmonitor_refresh_callback_slash(

>> +static int fsmonitor_refresh_callback_slash(
>>   	struct index_state *istate, const char *name, int len, int pos)
>>   {
> 
> This was split out a few patches ago, and the caller of course
> ignored the return value (void), but now it turns an integer, and
> this change is without a corresponding update to the caller, which
> leaves readers puzzled.
> 
> Perhaps a future patch either updates the existing caller or adds a
> new caller that utilize the returned value, but then at least the
> proposed commit message for this step should hint how it helps the
> caller(s) we will see in the future steps if this function returns
> the number of entries invalidated, iow, how the caller is expected
> to use the returned value from here, no?
> 
> Alternatively, this step can limit itself to what the commit title
> claims to do---to clarify what the helper does with enhanced in-code
> comments.  Then a future step that updates the caller to care about
> the return value can have both the changes to this callee as well as
> the caller---which may make it easier to see how the returned info
> helps the caller.  I dunno which is more reasonable.
>