Brandon Richardson <brandon1024...@gmail.com> writes:

> -             if (skip_prefix(arg, "-S", &sign_commit))
> +             if(!strcmp(arg, "--gpg-sign")) {

Style.  "if (!strcmp(arg, "--gpg-sign")) {"

> +                 skip_prefix(arg, "--gpg-sign", &sign_commit);
> +                 continue;

Technically, skipping the prefix S of string S will make us point at
an empty substring at the end.  So from that point of view,
skip_prefix(arg, "--gpg-sign", &sign_commit) is not incorrect
per-se, but it is highly misleading.  We have already determined
that the user gave us "--gpg-sign" option without anything after it,
so we want to summon the "use the default key" behaviour by giving
an empty string to sign_commit.

An explicit assignment

        sign_commit = "";

would be a lot more readable and make the intent a lot more clear.

> +             }
> +
> +             if (skip_prefix(arg, "-S", &sign_commit) ||
> +                     skip_prefix(arg, "--gpg-sign=", &sign_commit))

This side is OK.  "-S" gives us an empty string, but "-Skeyid" gives
us "keyid" in sign_commit.

>                       continue;
>  
>               if (!strcmp(arg, "--no-gpg-sign")) {
> diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh
> index 86d3f93fa..efc136eaf 100755
> --- a/t/t7510-signed-commit.sh
> +++ b/t/t7510-signed-commit.sh
> @@ -51,7 +51,9 @@ test_expect_success GPG 'create signed commits' '
>       # commit.gpgsign is still on but this must not be signed
>       git tag ninth-unsigned $(echo 9 | git commit-tree HEAD^{tree}) &&
>       # explicit -S of course must sign.
> -     git tag tenth-signed $(echo 9 | git commit-tree -S HEAD^{tree})
> +     git tag tenth-signed $(echo 10 | git commit-tree -S HEAD^{tree})
> +     # --gpg-sign must sign.
> +     git tag eleventh-signed $(echo 11 | git commit-tree --gpg-sign 
> HEAD^{tree})
>  '
>  
>  test_expect_success GPG 'verify and show signatures' '

Reply via email to