Re: [PATCH 1/3] merge: a random object may not necssarily be a commit

From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, Yann Droneaud <ydroneaud@opteya.com>
Subject: Re: [PATCH 1/3] merge: a random object may not necssarily be a commit
Date: Tue, 2 Apr 2013 11:03:44 -0400	[thread overview]
Message-ID: <20130402150343.GA18642@sigill.intra.peff.net> (raw)
In-Reply-To: <7vk3olgemi.fsf@alter.siamese.dyndns.org>

On Tue, Apr 02, 2013 at 08:02:13AM -0700, Junio C Hamano wrote:

> >> +	if (remote_head->util) {
> >> +		struct merge_remote_desc *desc;
> >> +		desc = merge_remote_util(remote_head);
> >> +		if (desc && desc->obj && desc->obj->type == OBJ_TAG) {
> >> +			strbuf_addf(msg, "%s\t\t%s '%s'\n",
> >> +				    sha1_to_hex(desc->obj->sha1),
> >> +				    typename(desc->obj->type),
> >> +				    remote);
> >> +			goto cleanup;
> >> +		}
> >> +	}
> >> +
> >>  	strbuf_addf(msg, "%s\t\tcommit '%s'\n",
> >>  		sha1_to_hex(remote_head->object.sha1), remote);
> >
> > I guess there is no other object type besides OBJ_TAG and OBJ_COMMIT
> > that would yield something we could merge, but it feels weird that you
> > check only for OBJ_TAG here, and otherwise still say "commit". Would the
> > intent be more clear if it just said:
> >
> >   if (desc && desc->obj && desc->obj->type != OBJ_COMMIT) {
> >           ...
> >
> > ?
> 
> I suspect not.
> 
> The point of the added code is that it knows we want to special case
> merging a tag object, and it wants to keep any other case behaving
> the same as before.

Ah. I read it as "if we have a random object, we do not want to just say
"commit X", because it is not a commit.

-Peff