Re: [PATCH] Documentation: clarify signature verification v2

From: Pranit Bauva <pranit.bauva@gmail.com>
Date: May 12, 2016, 08:04 UTC
Message-ID: <CAFZEwPOJg=njvhtK7ZQf_-3DUVjMpFBEV_0m1NHAE1UL7eM5ng@mail.gmail.com>
In-Reply-To: <20160512065022.GA32387@hashbang.sh>

On Thu, May 12, 2016 at 12:20 PM, Fox in the shell <KellerFuchs@hashbang.sh> wrote:

Show 6 quoted lines

>
> Hi,
>
> Here is a second attempt at this patch.
> Sorry for the delay, life somewhat got in the way.
>

Its okay! We understand that things might take a little more time than expected!

Show 11 quoted lines

> --
> Clarify which commits need to be signed.
>
> Uniformise the vocabulary used wrt. key/signature validity with OpenPGP:
> - a signature is valid if made by a key with a valid uid;
> - in the default trust-model, a uid is valid if signed by a trusted key;
> - a key is trusted if the (local) user set a trust level for it.
>
> Thanks to Junio C Hamano <gitster@pobox.com> for reviewing
>   the first attempt at this patch.
> ---

Its good to provide links to the previous version[1] of the patch. Please go through the Documentation/SubmittingPatches once. Seems like Junio was waiting for someone to point this out[2].

A couple of notes of how to submit the patches:
 * You have cc'ied the reviewers. Good!

 * Include the version no (v2) inside the [PATCH] like [PATCH v2]

 * The next version should be as a reply to the previous one.
    Hint: use --in-reply-to with git-send-email

 * git-am is used to pick up these patches and it gets the subject
   of the email and strips of [PATCH ...] and then uses the other stuff
   in the commit message headline.

 * The rest of the commit message are the words before ---.
    So currently git-am will pick up your paragraph as commit message:

          "Hi,

            Here is a second attempt at this patch.
            Sorry for the delay, life somewhat got in the way."

    which is quite undesirable as a commit message.

 * Comments are put after ---. So your paragraph
      "Clarify which commits need to be signed.

        Uniformise the vocabulary used wrt. key/signature validity with OpenPGP
         - a signature is valid if made by a key with a valid uid;
         - in the default trust-model, a uid is valid if signed by a
trusted key;
         - a key is trusted if the (local) user set a trust level for it.

           Thanks to Junio C Hamano <gitster@pobox.com> for reviewing
           the first attempt at this patch."

    is actually treated as a comment.

 * Also your signoff is missing.

 * If you want to credit someone then its better to use syntax like:
      "Helped-by: Junio C Hamano <gitster@pobox.com>"

 * It also seems like you probably wanted to add the
   "Reviewed-by:" tag. Please note only the reviewers can
   add that tag.

Show 39 quoted lines

>  Documentation/merge-options.txt  | 7 +++++--
>  Documentation/pretty-formats.txt | 4 ++--
>  2 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt
> index f08e9b8..30808a0 100644
> --- a/Documentation/merge-options.txt
> +++ b/Documentation/merge-options.txt
> @@ -89,8 +89,11 @@ option can be used to override --squash.
>
>  --verify-signatures::
>  --no-verify-signatures::
> -       Verify that the commits being merged have good and trusted GPG signatures
> -       and abort the merge in case they do not.
> +       Verify that the tip commit of the side branch being merged is
> +       signed with a valid key, i.e. a key that has a valid uid: in the
> +       default trust model, this means the signing key has been signed by
> +       a trusted key.  If the tip commit of the side branch is not signed
> +       with a valid key, the merge is aborted.
>
>  --summary::
>  --no-summary::
> diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
> index 671cebd..29b19b9 100644
> --- a/Documentation/pretty-formats.txt
> +++ b/Documentation/pretty-formats.txt
> @@ -143,8 +143,8 @@ ifndef::git-rev-list[]
>  - '%N': commit notes
>  endif::git-rev-list[]
>  - '%GG': raw verification message from GPG for a signed commit
> -- '%G?': show "G" for a Good signature, "B" for a Bad signature, "U" for a good,
> -  untrusted signature and "N" for no signature
> +- '%G?': show "G" for a good (valid) signature, "B" for a bad signature,
> +  "U" for a good signature with unknown validity and "N" for no signature
>  - '%GS': show the name of the signer for a signed commit
>  - '%GK': show the key used to sign a signed commit
>  - '%gD': reflog selector, e.g., `refs/stash@{1}`
> --
> 2.1.4

[1]: http://thread.gmane.org/gmane.comp.version-control.git/291123 [2]: http://article.gmane.org/gmane.comp.version-control.git/291185