From: Jeff King <peff@peff.net>
Date: Wed, 18 Feb 2015 18:50:08 GMT
Subject: Re: [PATCH 2/2] builtin/push.c: make push_default a static variable
Message-ID: <20150218185007.GA7257@peff.net>
In-Reply-To: <xmqqzj8cyyip.fsf@gitster.dls.corp.google.com>

On Tue, Feb 17, 2015 at 02:16:30PM -0800, Junio C Hamano wrote:

> Do you want to resurrect that @{publish} stuff?  I think it had
> sensible semantics, and I do not think we mind keeping the
> push_default configuration to be read from the default_config
> codepath.

I'll take a look at it and see if it's in good enough shape to apply
as-is, or with minor tweaking. But regardless, let's...

> If we decide to go that route, then the series would become
> something like this:
> 
> $gmane/263871 [1/4] git_push_config: drop cargo-culted wt_status pointer
> $gmane/263878 [2/4] cmd_push: set "atomic" bit directly
> $gmane/263879 [3/4] cmd_push: pass "flags" pointer to config callback
> $gmane/263880 [4/4] push: allow --follow-tags to be set by config push.followTags
> 
> omitting the original 2/2 patch we are discussing.  I am inclined to
> replace what I queued with the above four.

...do this. Even if we don't apply other patches to make use of
push_default immediately, it's a plausible area for us to touch later,
and the cleanup from the dropped patch was not so important.

> +	if (!strcmp(k, "push.followtags")) {
> +		if (git_config_bool(k, v))
> +			*flags |= TRANSPORT_PUSH_FOLLOW_TAGS;
> +		else
> +			*flags &= ~TRANSPORT_PUSH_FOLLOW_TAGS;
> +		return 0;
> +	}

Did you have an opinion on sticking this behind a helper function?

It feels like a lot of repeating of the same variables and flags, but I
worried that "munge_bit" ends up being even more confusing.

-Peff