From: Jeff King <peff@peff.net>
Date: Fri, 09 Sep 2016 20:37:42 GMT
Subject: Re: [PATCH v3 2/2] patch-ids: define patch-id of merge commits as "null"
Message-ID: <20160909203742.cyg3y7ju6vhfrx7o@sigill.intra.peff.net>
In-Reply-To: <20160909203447.nlyx6v76fpyx2vzl@sigill.intra.peff.net>

On Fri, Sep 09, 2016 at 04:34:47PM -0400, Jeff King wrote:

> This patch defines the patch-id of a merge commit as
> essentially "null"; it has no patch-id. As a result,
> merges cannot match patch-ids via "--cherry-pick", and
> "format-patch --base" will not list merges in its list of
> prerequisite patch ids.
> 
> To distinguish between real errors and "null", we have to
> expand the semantics of commit_patch_id()'s return value,
> and callers need to distinguish these cases.

One alternative would be to add an out-parameter that is set in the
success case saying "yes, we have a real patch-id". And then the callers
could look like:

  if (commit_patch_id(commit, &diffopt, sha1, 0, &got_one))
	die("error!");
  if (!got_one)
	continue; /* silently skip */

We could even use the null sha1 to signal that rather than an extra
parameter, I suppose.

I dunno. It would make the callers less clunky, I think, but it does
feel a bit magical.

-Peff