All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Sunshine <sunshine@sunshineco.com>
To: Santiago Torres <santiago@nyu.edu>
Cc: Git List <git@vger.kernel.org>,
	Junio C Hamano <gitster@pobox.com>, Jeff King <peff@peff.net>
Subject: Re: [PATCH v7 4/6] verify-tag: prepare verify_tag for libification
Date: Tue, 19 Apr 2016 15:05:01 -0400	[thread overview]
Message-ID: <CAPig+cTO_vC1_N79G9yMkiyjYtEeBdsivz-A4jOMw0r-vRWWaw@mail.gmail.com> (raw)
In-Reply-To: <1461088041-19264-5-git-send-email-santiago@nyu.edu>

On Tue, Apr 19, 2016 at 1:47 PM,  <santiago@nyu.edu> wrote:
> The current interface of verify_tag() resolves reference names to SHA1,
> however, the plan is to make this functionality public and the current
> interface is cumbersome for callers: they are expected to supply the
> textual representation of a sha1/refname. In many cases, this requires
> them to turn the sha1 to hex representation, just to be converted back
> inside verify_tag.
>
> Add a SHA1 parameter to use instead of the name parameter, and rename
> the name parameter to "name_to_report" for reporting purposes only.

I'd have probably called this "display_name", but then I suppose it
suffers the same issue Junio mentioned previously about it sounding
like a boolean. Anyhow, as long as Junio is happy with it, that's what
matters.

This version of the patch is nicely improved. One nit below.

> Signed-off-by: Santiago Torres <santiago@nyu.edu>
> ---
> diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
> @@ -80,6 +83,8 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix)
>  {
>         int i = 1, verbose = 0, had_error = 0;
>         unsigned flags = 0;
> +       unsigned char sha1[20];
> +       const char *name;

Mentioned previously[1]: These two declarations could be moved inside
the while-loop scope (below).

[1]: http://article.gmane.org/gmane.comp.version-control.git/291813

>         const struct option verify_tag_options[] = {
>                 OPT__VERBOSE(&verbose, N_("print tag contents")),
>                 OPT_BIT(0, "raw", &flags, N_("print raw gpg status output"), GPG_VERIFY_RAW),
> @@ -96,8 +101,12 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix)
>         if (verbose)
>                 flags |= GPG_VERIFY_VERBOSE;
>
> -       while (i < argc)
> -               if (verify_tag(argv[i++], flags))
> +       while (i < argc) {
> +               name = argv[i++];
> +               if (get_sha1(name, sha1))
> +                       had_error = !!error("tag '%s' not found.", name);
> +               else if (verify_tag(sha1, name, flags))
>                         had_error = 1;
> +       }
>         return had_error;
>  }
> --
> 2.8.0

  reply	other threads:[~2016-04-19 19:05 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-19 17:47 [PATCH v7 0/6] Move PGP verification out of verify-tag santiago
2016-04-19 17:47 ` [PATCH v7 1/6] builtin/verify-tag.c: ignore SIGPIPE in gpg-interface santiago
2016-04-19 17:47 ` [PATCH v7 2/6] t7030: test verifying multiple tags santiago
2016-04-19 17:47 ` [PATCH v7 3/6] verify-tag: update variable name and type santiago
2016-04-19 17:47 ` [PATCH v7 4/6] verify-tag: prepare verify_tag for libification santiago
2016-04-19 19:05   ` Eric Sunshine [this message]
2016-04-19 19:36     ` Junio C Hamano
2016-04-19 17:47 ` [PATCH v7 5/6] verify-tag: move tag verification code to tag.c santiago
2016-04-19 17:47 ` [PATCH v7 6/6] tag -v: verfy directly rather than exec-ing verify-tag santiago
2016-04-19 19:06   ` Eric Sunshine
2016-04-19 19:10 ` [PATCH v7 0/6] Move PGP verification out of verify-tag Eric Sunshine

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAPig+cTO_vC1_N79G9yMkiyjYtEeBdsivz-A4jOMw0r-vRWWaw@mail.gmail.com \
    --to=sunshine@sunshineco.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    --cc=santiago@nyu.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.