Subject: Re: [PATCH v2] remote: prefetch config

From: Junio C Hamano <[email-masked]>
Date: Thu, 05 Sep 2024 10:54:33 -0700

Shubham Kanodia <[email-masked]> writes:

> On Thu, Sep 5, 2024 at 10:22 PM Junio C Hamano <[email-masked]> wrote:
>>
>> Shubham Kanodia <[email-masked]> writes:
>>
>> >>
>> >>         int prefetch; /* is prefetch enabled? */
>> >>
>> > ...
>> > Updating my patch — please let me know if there's anything else I can
>> > improve here.
>>
>> Renaming the .prefetch member to .prefetch_enabled would eliminate
>> the need to add any comment on the member in the header file.
>
> Do you mean for the struct member here or also the config?

I do not think I mentioned anything about the name of the
configuration variable, but if I did that was a mistake.

End-user facing configuration variables are often named after a
feature that it enables or disables, so it can use the name without
"enable".  An int variable on the other hand can mean many other
things, ranging from "how many times have we prefetched from here"
to "does this remote allow prefetching?", so a more explicit name
would often help.

Thanks.