Re: [PATCH v2 1/8] environment: move "trust_ctime" into `struct repo_config_values`
Karthik Nayak <karthik.188@gmail.com> writes:
Show 21 quoted lines
>>>> Store it instead in `repo_config_values`, so the value is tied to the
>>>> repository from which it was read. This preserves existing behavior
>>>> while avoiding cross-repository state leakage and continues the effort
>>>> to reduce reliance on global configuration state.
>>>>
>>>> Update all references to use repo_config_values().
>>>>
>>>
>>> Nit: I was hoping you'd also shed light on why this can go into
>>> `repo_config_values()`. Does it need to be eagerly parsed? If so, why?
>>
>> If trust_ctime was lazily parsed where it is used we'd end up dying in
>> match_stat_data() which would be quite unexpected, make it very hard to
>> reason about the code, and hamper the libification efforts. I'd much
>> rather we put the onus on patch authors to justify any conversion from
>> eager parsing to lazy parsing rather than forcing them to justify
>> continuing to parse settings eagerly.
>>
>
> Agreed. A note in the commit message that this belongs in
> `repo_config_values()` because it's eagerly parsed would be enough.
I see "preserves existing behavior" above. Wouldn't it be enough?