Re: [PATCH 1/1] config: support a default remote tracking setup upon branch creation

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 30, 2021, 17:32 UTC
Message-ID: <xmqqim0r1ze0.fsf@gitster.g>
In-Reply-To: <bcbe13a8-7f22-7564-77cd-674c7b2acbf6@gmail.com>

Philippe Blain <levraiphilippeblain@gmail.com> writes:

Show 10 quoted lines

>> diff --git a/environment.c b/environment.c
>> index 2f27008424..d550deabbd 100644
>> --- a/environment.c
>> +++ b/environment.c
>> @@ -60,6 +60,8 @@ int global_conv_flags_eol = CONV_EOL_RNDTRP_WARN;
>>   char *check_roundtrip_encoding = "SHIFT-JIS";
>>   unsigned whitespace_rule_cfg = WS_DEFAULT_RULE;
>>   enum branch_track git_branch_track = BRANCH_TRACK_REMOTE;
>> +const char* git_branch_remote = NULL;
>> +const char* git_branch_merge = NULL;

Style:
 (1) asterisk sticks to the identifier, not type, in our codebase.
 (2) do not initialize globals and statics to 0 or NULL.

> Can the new settings be implemented without adding more global variables ?

This is worth considering in the longer term. For things like these new configuration items and existign git_branch_track, we already have reasonably made abstraction that branch.c is where interesting actions happen (like setting up remote tracking, etc), so there is no reason for them to be in environment.c or *.h to be visible to anywhere outside branch.c file.

I wonder if it is a matter of moving git_default_branch_config() to branch.c from config.c and make it global, while moving these global variables also to branch.c and make them file-local?

I am still unsure without the expected use-case well documented, if it is clear enough for users to learn how and when these new configurations should be used (as opposed to following the traditional triangular workflow) with only the documentation updates in this patch, but at least I can trust others like you to give input to polish this into a reasonable shape.

Thanks for a review (and thanks for starting the effort, Ben).