Re: [PATCH v8 2/2] format-patch --signature-file file

[Junio C Hamano]

Jeremiah Mahler jmmah...@gmail.com writes:

 Your comments make it clear that I have not accounted for all the possible
 cases.  Below is a table of all the reasonable cases.  It should account
 for cases you mentioned as well as others.

   Key:
   ---
   default: Git version number
   sig1: .signature from column 1
   file1: .signaturefile from column 1
   sig2: --signature from column 2
   file2: --signature-file in column 2

 A preceeding 'format.' is assumed for config.  .signature - format.signature
 Command line arguments take precedence over config options.

;-)

I do not mind the full matrix if it is cleanly done.

It may be an overkill and a maintenance burden, especially if it can
be done only with a sequence of cut-and-paste repetition (i.e. not
as a loop in the test script that synthesises the permutations).

The suggestion for those three combinations were meant to be an
easy-to-implement-and-maintain compromise.

Thanks.

 |+-+-|
 | config  (1)| argv  (2)   | |
 |+-+-|
 || | default |
 || --signature | sig2|
 || --signature-file| file2   |
 || --no-signature  | none|
 || --no-signature-file | none|
 || --signature, --signature-file   | die |
 || --signature, --no-signature-file| sig2|
 || --signature-file, --no-signature| none|
 || --no-signature, --no-signature-file | none|
 | .signature | | sig1|
 | .signature | --signature | sig2|
 | .signature | --signature-file| file2   |
 | .signature | --no-signature  | none|
 | .signature | --no-signature-file | sig1|
 | .signature | --signature, --signature-file   | die |
 | .signature | --signature, --no-signature-file| sig2|
 | .signature | --signature-file, --no-signature| none|
 | .signature | --no-signature, --no-signature-file | none|
 | .signaturefile | | file1   |
 | .signaturefile | --signature | sig2|
 | .signaturefile | --signature-file| file2   |
 | .signaturefile | --no-signature  | none|
 | .signaturefile | --no-signature-file | default |
 | .signaturefile | --signature, --signature-file   | die |
 | .signaturefile | --signature, --no-signature-file| sig2|
 | .signaturefile | --signature-file, --no-signature| none|
 | .signaturefile | --no-signature, --no-signature-file | none|
 | .signature, .signaturefile | | die |
 | .signature, .signaturefile | --signature | sig2|
 | .signature, .signaturefile | --signature-file| file2   |
 | .signature, .signaturefile | --no-signature  | none|
 | .signature, .signaturefile | --no-signature-file | sig1|
 | .signature, .signaturefile | --signature, --signature-file   | die |
 | .signature, .signaturefile | --signature, --no-signature-file| sig2|
 | .signature, .signaturefile | --signature-file, --no-signature| none|
 | .signature, .signaturefile | --no-signature, --no-signature-file | none|
 |+-+-|


 Thanks,
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 2/2] format-patch --signature-file file

[Junio C Hamano]

Jeremiah Mahler jmmah...@gmail.com writes:

 Added option that allows a signature file to be used with format-patch
 so that signatures with newlines and other special characters can be
 easily included.

s/Added option/Add an option/.  I do not think with newlines and
other special characters is the primary issue---isn't it more about
I have chosen to use this mail-signature; do not force me to retype
the same all the time?

   $ git format-patch --signature-file ~/.signature -1

The recommended command-line convention (see gitcli(7)) is to use
--option=value, so an example would be better to follow it, i.e.

$ git format-patch -1 --signature-file=$HOME/.signature

 The config variable format.signaturefile is also provided so that it
 can be added by default.

   $ git config format.signaturefile ~/.signature
   $ git format-patch -1

Something like:

To countermand the configuration variable for a specific run:

$ git format-patch -1 --signature=This time only
$ git format-patch -1 --signature;# to use the default
$ git format-patch -1 --signature= ;# to add nothing

is also needed here, I think.  Similarly, these two needs to be
tested in the test scripts you are modifying.  Specifically:

 +test_expect_success 'format-patch --no-signature and --signature-file OK' '
 + git format-patch --stdout --no-signature 
 --signature-file=mail-signature -1
 +'

should not just make sure format-patch does _something_, but needs
to make sure it does not contain the contents of the configured mail
signagture file.

I didn't see offhand if the tests make sure that a configured mail
signature can be overriden from the command line.  I think you would
want to test, with format-patch.signature-file pointing at the
mail-signature file, at least these three cases:

 - Run format-patch --no-signature and make sure that stops the
   contents from mail-signature file from being shown, and instead
   no mail-signature is given.

 - Run format-patch --signature='this time only' and make sure
   that stops the contents from mail-signature file from being shown
   and this time only is used instead.

 - Run format-patch --signature-file=another-mail-signature and
   make sure that stops the contents from mail-signature file from
   being shown and the contents from the other file is used instead.

Test for these negative cases is often what we forget, when we are
thrilled to show off that the shiny new feature works as expected.
We need to ensure that the ways to stop the shiny new feature from
kicking in will not be broken as well.

Thanks.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html