Re: [PATCH 3/3] environment: reorder variables in repo_config_values structure

On 8/7/26 00:42, Junio C Hamano wrote:
> Tian Yuchen <[email protected]> writes:
> 
>> On 8/6/26 05:47, Junio C Hamano wrote:
>>> Tian Yuchen <[email protected]> writes:
>>>
>>>> Reorder the fields in struct repo_config_values and its initialization
>>>> function to follow the order of configuration sections.
>>>>
>>>> Keeping the declaration and initialization order aligned makes the
>>>> structure easier to review and maintain.
>>>
>>> Really?
>>>
>>> Do you have some automated tool to make sure these initialization
>>> assignments in the environment.c file and declaration in the
>>> environment.h file match the order in Documentation/config/*.adoc or
>>> something else?  Have you designated some list as the authoritative
>>> source of truth to check these against?  Without such a list to
>>> check the code against and a mechanism to enforce the ordering, I
>>> find it hard to agree with such a claim that this makes it easier to
>>> maintain.
>>
>> I see.
>>
>>>
>>> It is typical to list the structure members in the order of stricter
>>> to looser alignment requirement of their types.  I do not know how
>>> strictly it is followed for "struct repo_config_values", but by
>>> spreading pointer valued members more widely with smaller enums in
>>> between, the change certainly is making the overall structure size
>>> larger by requiring more padding between the members with different
>>> alignment requirements.  Not that we would have 100s of instances of
>>> these structures.
>>>
>>
>> Oh, I overlooked the size issue. Thanks for pointing out.
> 
> I didn't mean to "point out" any size issue.  As I said, it is not
> like we have hundreds of these, so padding bloat here and there
> would not matter and if we get a readability boost by reordering
> into a sensible order, that by itself could be a win.
> 

Okay.

As you said before, the boost on readability seems to be limited. 
Reordering by config section is not a strong maintenance rule without an 
authoritative source. So let's don't reorder them anyways.

Thanks, yuchen