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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Philippe Blain <levraiphilippeblain@xxxxxxxxx> writes:

>> 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).







[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux