From: Josh Steadmon <steadmon@google.com>
Date: Tue, 14 Dec 2021 20:37:18 GMT
Subject: Re: [PATCH v5 2/2] branch: add flags and config to inherit tracking
Message-ID: <YbkAfk1r/ONeYyIN@google.com>
In-Reply-To: <xmqq7dcg6w43.fsf@gitster.g>

On 2021.12.07 11:41, Junio C Hamano wrote:
> Josh Steadmon <steadmon@google.com> writes:
> 
> >  --no-track::
> >  	Do not set up "upstream" configuration, even if the
> > -	branch.autoSetupMerge configuration variable is true.
> > +	branch.autoSetupMerge configuration variable is set.
> 
> I guess "inherit" is different from "true".
> Nice to see an attention to the details.  
> 
> > diff --git a/branch.h b/branch.h
> > index df0be61506..6484bda8a2 100644
> > --- a/branch.h
> > +++ b/branch.h
> > @@ -10,7 +10,8 @@ enum branch_track {
> >  	BRANCH_TRACK_REMOTE,
> >  	BRANCH_TRACK_ALWAYS,
> >  	BRANCH_TRACK_EXPLICIT,
> > -	BRANCH_TRACK_OVERRIDE
> > +	BRANCH_TRACK_OVERRIDE,
> > +	BRANCH_TRACK_INHERIT
> >  };
> 
> Unless INHERIT must stay to be at the end of the enumeration even
> when we add more of these, let's leave a common at the end to help
> future developers.

Fixed in V6.


> > -		if (value && !strcasecmp(value, "always")) {
> > +		if (value && !strcmp(value, "always")) {
> 
> This is belatedly correcting the mistake we made 5 years ago, which
> can break existing users who used "[branch]autosetupmerge=Always" in
> their configuration files.
> 
> I'm OK to fix it to accept only lowercase as written here, see if
> anybody screams, and tell them that the all-lowercase is the only
> documented way to spell this word.
> 
> >  			git_branch_track = BRANCH_TRACK_ALWAYS;
> >  			return 0;
> > +		} else if (value && !strcmp(value, "inherit")) {
> > +			git_branch_track = BRANCH_TRACK_INHERIT;
> > +			return 0;
> >  		}
> >  		git_branch_track = git_config_bool(var, value);
> >  		return 0;
> 
> Thanks.