Re: [PATCH v2] Make 'trust_executable_bit' repository-scoped

[Junio C Hamano <gitster@pobox.com>]

cat@malon.dev writes:

>> Hi drona,
>> 
>> Thanks for the update! Just a quick heads-up: it looks like
>> you forgot to CC Junio (gitster@pobox.com) on this iteration.

No strong need to Cc the maintainer when the patch is not ready to
be applied, even though it may be nice.  I'll be seeing it either
way as I rarely look at my mailbox and use the mailing list archive
at lore.kernel.org my primary source of Git patches anyway.

There were discussions on pros and cons moving global recipients of
configuration values into a dynamically allocated strucrure, which
can change when they are parsed and when bad values in them result
in warnings, depending on the way the change is done, and excellent
pieces of advice have been given by Phillip Wood.  If anything, a
change like this should ask for input from him.

>> Additionally, I think it's a good practice to respond to
>> reviews before sending new patches.

Absolutely.

>>>  	if (!strcmp(var, "core.filemode")) {
>>> +		prepare_repo_settings(the_repository);
>>>  		the_repository->settings.trust_executable_bit = 
>>> git_config_bool(var, value);
>>>  		return 0;
>>>  	}
>> 
>> Regarding the code, calling 'prepare_repo_settings()' inside
>> 'git_default_core_config()' defeats the purpose of lazy-loading,
>> doesn't it?
>> 
>> if (!strcmp(var, "core.filemode")) {
>> 	prepare_repo_settings(the_repository);
>> 	the_repository->settings.trust_executable_bit = git_config_bool(var, 
>> value);
>> 	return 0;
>> }
>> 
>> I think the standard practice is to drop the variable from
>> 'environment.c' completely and read it directly inside
>> 'repo-settings.c: prepare_repo_settings()' using
>> 'repo_config_get_bool()'.

This "v2" applies to a mythical codebase where trust_executable_bit
is somehow a member in the settings structure, which I do not think
we have.