Re: [PATCH v3 4/4] tag: use pgp_verify_function in tag -v call

From: Jeff King <peff@peff.net>
Date: Apr 4, 2016, 13:38 UTC
Message-ID: <20160404133853.GB25404@sigill.intra.peff.net>
In-Reply-To: <20160404041203.GE28933@LykOS>

On Mon, Apr 04, 2016 at 12:12:04AM -0400, Santiago Torres wrote:

Show 8 quoted lines

> > As a side note, it might actually be an improvement for pgp_verify_tag
> > to take a sha1 (so that git-tag is sure that it is verifying the same
> > object that it is printing), but that refactoring should probably come
> > separately, I think.
> 
> Just to be sure, this refactoring is something we should still include
> in this set of patches, right? I think that otherwise we'd lose the
> desambigutaion that git tag -v does in this patch.

I think it can be part of this series, but doesn't have to be. As I understand it, the current code is just handing the name to the `git verify-tag` process, so if we continue to do so, that would be OK.

> I also think that most of the rippling is gone if we use and adaptor as
> you suggested. Should I add a patch on top of this to support a sha1 as
> part for gpg_verify_tag()?

Yes, though I'd generally advise against a function taking either a name or a sha1, and ignoring the other option. That often leads to confusing interfaces for the callers. Instead, perhaps just take the sha1, and let the caller do the get_sha1() themselves. Or possibly provide two functions, one of which is a convenience to translate the name to sha1 and then call the other.

-Peff