All of lore.kernel.org
 help / color / mirror / Atom feed
From: Denton Liu <liu.denton@gmail.com>
To: Git Mailing List <git@vger.kernel.org>
Cc: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Junio C Hamano" <gitster@pobox.com>,
	"Eric Sunshine" <sunshine@sunshineco.com>
Subject: [PATCH v3 00/13] format-patch: learn --infer-cover-subject option (also t4014 cleanup)
Date: Tue, 20 Aug 2019 03:18:43 -0400	[thread overview]
Message-ID: <cover.1566285151.git.liu.denton@gmail.com> (raw)
In-Reply-To: <cover.1566258525.git.liu.denton@gmail.com>

Thanks for another round of reviews, Eric. I've incorporated most of
your suggestions, including breaking the t4014 cleanup patch into
multiple patches. Hopefully it isn't such a doozy to review now.

Currently, format-patch only puts "*** SUBJECT HERE ***" when a cover
letter is generated. However, it is already smart enough to be able to
populate the cover letter with the branch description so there's no
reason why it cannot populate the subject as well.

Teach format-patch the `--infer-cover-subject` option and corresponding
`format.inferCoverSubject` configuration option which will read the
subject from the branch description using the same rules as for a commit
message (that is, it will expect a subject line followed by a blank
line).

While we're at it, perform some major cleanup of t4014 including some
stylistic cleanup and also, unmasking of Git return codes.

This was based on patches 1-3 of an earlier patchset I sent[1].

Changes since v2:

* Break 1/4 into many different patches (one per paragraph of the
  original patch)

* Incorporate Eric's documentation/commit message suggestions

Changes since v1:

* Incorporate Eric's suggestions for cleanup in all patches

* Add patch 3/4 to make it clear what is the default value for
  format.coverLetter (since format.inferCoverSubject was borrowed from
  this config but it also did not state what the default value was)

* In 1/4, rename all instances of "expected" to "expect"

[1]: https://public-inbox.org/git/cover.1558492582.git.liu.denton@gmail.com/


Denton Liu (13):
  t4014: drop unnecessary blank lines from test cases
  t4014: s/expected/expect/
  t4014: move closing sq onto its own line
  t4014: use sq for test case names
  t4014: remove spaces after redirect operators
  t4014: use indentable here-docs
  t4014: drop redirections to /dev/null
  t4014: use test_line_count() where possible
  t4014: remove confusing pipe in check_threading()
  t4014: stop losing return codes of git commands
  Doc: add more detail for git-format-patch
  config/format.txt: specify default value of format.coverLetter
  format-patch: learn --infer-cover-subject option

 Documentation/config/format.txt    |   7 +
 Documentation/git-format-patch.txt |  24 +-
 builtin/log.c                      |  56 +-
 t/t4014-format-patch.sh            | 822 +++++++++++++++--------------
 4 files changed, 486 insertions(+), 423 deletions(-)

Range-diff against v2:
 1:  76a0a274fd <  -:  ---------- t4014: clean up style
 -:  ---------- >  1:  fb000bfca2 t4014: drop unnecessary blank lines from test cases
 -:  ---------- >  2:  568b3a03a0 t4014: s/expected/expect/
 -:  ---------- >  3:  a205a920bd t4014: move closing sq onto its own line
 -:  ---------- >  4:  66bf2e3dd4 t4014: use sq for test case names
 -:  ---------- >  5:  6f1371275e t4014: remove spaces after redirect operators
 -:  ---------- >  6:  b4295846f5 t4014: use indentable here-docs
 -:  ---------- >  7:  34315412c8 t4014: drop redirections to /dev/null
 -:  ---------- >  8:  de08dd886d t4014: use test_line_count() where possible
 -:  ---------- >  9:  dec5a62e82 t4014: remove confusing pipe in check_threading()
 -:  ---------- > 10:  64069c0c54 t4014: stop losing return codes of git commands
 2:  fd908bcc01 ! 11:  c12534ab5d Doc: add more detail for git-format-patch
    @@ Commit message
         Doc: add more detail for git-format-patch
     
         In git-format-patch.txt, we were missing some key user information.
    -    First of all, using the `--to` and `--cc` options don't override
    -    `format.to` and `format.cc` variables, respectively. They add on to each
    -    other. Document this.
    -
    -    In addition, document the special value of `--base=auto`.
    +    First of all, document the special value of `--base=auto`.
     
         Next, while we're at it, surround option arguments with <>.
     
 3:  94a778c9aa <  -:  ---------- config/format.txt: make clear the default value of format.coverLetter
 -:  ---------- > 12:  a08273ebcc config/format.txt: specify default value of format.coverLetter
 4:  e682bd347a ! 13:  de599f7ca9 format-patch: learn --infer-cover-letter option
    @@ Metadata
     Author: Denton Liu <liu.denton@gmail.com>
     
      ## Commit message ##
    -    format-patch: learn --infer-cover-letter option
    +    format-patch: learn --infer-cover-subject option
     
    -    We used to populate the subject of the cover letter generated by
    -    git-format-patch with "*** SUBJECT HERE ***". However, if a user submits
    -    multiple patchsets, they may want to keep a consistent subject between
    -    rerolls.
    +    Teach format-patch to use the first line of the branch description as
    +    the Subject: of the generated cover letter, rather than "*** SUBJECT
    +    HERE ***" if `--infer-cover-subject` is specified or the corresponding
    +    `format.inferCoverSubject` option is enabled. This complements the
    +    existing inclusion of the branch description in the cover letter body.
     
    -    If git-format-patch is run with `--infer-cover-letter` or
    -    `format.inferCoverSubject`, infer the subject for the cover letter from
    -    the top line(s) of a branch description, similar to how a subject is
    -    read from a commit message.
    +    The reason why this behaviour is not made default is because this change
    +    is not backwards compatible and may break existing tooling that may rely
    +    on the default template subject.
     
         Signed-off-by: Denton Liu <liu.denton@gmail.com>
     
    @@ Documentation/config/format.txt: format.subjectPrefix::
      	subject prefix. Use this variable to change that prefix.
      
     +format.inferCoverSubject::
    -+	A boolean value which lets you enable the
    -+	`--infer-cover-subject` option of format-patch by default.
    ++	A boolean that controls whether or not to take the first line of
    ++	the branch description as the subject for the cover letter. See the
    ++	`--infer-cover-subject` option in linkgit:git-format-patch[1].
    ++	Default is false.
     +
      format.signature::
      	The default for format-patch is to output a signature containing
    @@ Documentation/git-format-patch.txt: will want to ensure that threading is disabl
      	ignored.
      
     +--[no-]infer-cover-subject::
    -+	Instead of using the default "*** SUBJECT HERE ***" subject for
    -+	the cover letter, infer the subject from the branch's
    -+	description.
    -++
    -+Similar to a commit message, the subject is inferred as the beginning of
    -+the description up to and excluding the first blank line.
    ++	Use the beginning of the branch description (up to the first
    ++	blank line) as the cover letter subject instead of the default
    ++	"*** SUBJECT HERE ***".
     +
      --subject-prefix=<Subject-Prefix>::
      	Instead of the standard '[PATCH]' prefix in the subject
    @@ builtin/log.c: int cmd_format_patch(int argc, const char **argv, const char *pre
      			    N_("Use [RFC PATCH] instead of [PATCH]"),
      			    PARSE_OPT_NOARG | PARSE_OPT_NONEG, rfc_callback },
     +		OPT_BOOL(0, "infer-cover-subject", &infer_cover_subject,
    -+			    N_("infer a cover letter subject from the branch description")),
    ++			    N_("infer a cover letter subject from branch description")),
      		{ OPTION_CALLBACK, 0, "subject-prefix", &rev, N_("prefix"),
      			    N_("Use [<prefix>] instead of [PATCH]"),
      			    PARSE_OPT_NONEG, subject_prefix_callback },
    @@ builtin/log.c: int cmd_format_patch(int argc, const char **argv, const char *pre
      		total++;
     
      ## t/t4014-format-patch.sh ##
    +@@ t/t4014-format-patch.sh: test_expect_success 'format-patch --ignore-if-in-upstream HEAD' '
    + '
    + 
    + test_expect_success 'get git version' '
    +-	git_version="$(git --version | sed "s/.* //")"
    ++	git_version="$(git --version >version && sed "s/.* //" <version)"
    + '
    + 
    + signature() {
     @@ t/t4014-format-patch.sh: test_expect_success 'format patch ignores color.ui' '
      	test_cmp expect actual
      '
-- 
2.23.0.248.g3a9dd8fb08


  parent reply	other threads:[~2019-08-20  7:18 UTC|newest]

Thread overview: 91+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-19 23:52 [PATCH v2 0/4] format-patch: learn --infer-cover-subject option Denton Liu
2019-08-19 23:52 ` [PATCH v2 1/4] t4014: clean up style Denton Liu
2019-08-20  2:41   ` Eric Sunshine
2019-08-19 23:52 ` [PATCH v2 2/4] Doc: add more detail for git-format-patch Denton Liu
2019-08-20  2:44   ` Eric Sunshine
2019-08-20  7:07     ` Denton Liu
2019-08-19 23:52 ` [PATCH v2 3/4] config/format.txt: make clear the default value of format.coverLetter Denton Liu
2019-08-20  2:47   ` Eric Sunshine
2019-08-19 23:52 ` [PATCH v2 4/4] format-patch: learn --infer-cover-letter option Denton Liu
2019-08-20  3:46   ` Eric Sunshine
2019-08-20  7:18 ` Denton Liu [this message]
2019-08-20  7:18   ` [PATCH v3 01/13] t4014: drop unnecessary blank lines from test cases Denton Liu
2019-08-20  7:18   ` [PATCH v3 02/13] t4014: s/expected/expect/ Denton Liu
2019-08-20 21:31     ` Eric Sunshine
2019-08-20  7:18   ` [PATCH v3 03/13] t4014: move closing sq onto its own line Denton Liu
2019-08-20  7:18   ` [PATCH v3 04/13] t4014: use sq for test case names Denton Liu
2019-08-20  7:18   ` [PATCH v3 05/13] t4014: remove spaces after redirect operators Denton Liu
2019-08-20  7:18   ` [PATCH v3 06/13] t4014: use indentable here-docs Denton Liu
2019-08-20  7:19   ` [PATCH v3 07/13] t4014: drop redirections to /dev/null Denton Liu
2019-08-20  7:19   ` [PATCH v3 08/13] t4014: use test_line_count() where possible Denton Liu
2019-08-20  7:19   ` [PATCH v3 09/13] t4014: remove confusing pipe in check_threading() Denton Liu
2019-08-20  7:19   ` [PATCH v3 10/13] t4014: stop losing return codes of git commands Denton Liu
2019-08-20  7:31     ` Denton Liu
2019-08-20 19:04       ` Johannes Sixt
2019-08-20  7:19   ` [PATCH v3 11/13] Doc: add more detail for git-format-patch Denton Liu
2019-08-21 18:26     ` Junio C Hamano
2019-08-20  7:19   ` [PATCH v3 12/13] config/format.txt: specify default value of format.coverLetter Denton Liu
2019-08-20  7:19   ` [PATCH v3 13/13] format-patch: learn --infer-cover-subject option Denton Liu
2019-08-21 19:32     ` Junio C Hamano
2019-08-23 18:15       ` Denton Liu
2019-08-23 18:46         ` Philip Oakley
2019-08-23 20:18           ` Junio C Hamano
2019-08-24  8:03             ` Denton Liu
2019-08-24 13:59               ` Philip Oakley
2019-08-26 14:30                 ` Junio C Hamano
2019-08-26 14:26               ` Junio C Hamano
2019-08-26 16:05                 ` Junio C Hamano
2019-08-22 20:18   ` [PATCH v3 00/13] format-patch: learn --infer-cover-subject option (also t4014 cleanup) Junio C Hamano
2019-08-23 18:19     ` Denton Liu
2019-08-23 20:25       ` Junio C Hamano
2019-08-24  8:25   ` [PATCH 00/13] format-patch: clean up tests and documentation Denton Liu
2019-08-24  8:26     ` [PATCH 01/13] t4014: drop unnecessary blank lines from test cases Denton Liu
2019-08-24  8:26     ` [PATCH 02/13] t4014: s/expected/expect/ Denton Liu
2019-08-24  8:26     ` [PATCH 03/13] t4014: move closing sq onto its own line Denton Liu
2019-08-24  8:26     ` [PATCH 04/13] t4014: use sq for test case names Denton Liu
2019-08-24  8:26     ` [PATCH 05/13] t4014: remove spaces after redirect operators Denton Liu
2019-08-24  8:27     ` [PATCH 06/13] t4014: use indentable here-docs Denton Liu
2019-08-24  8:27     ` [PATCH 07/13] t4014: drop redirections to /dev/null Denton Liu
2019-08-24  8:27     ` [PATCH 08/13] t4014: let sed open its own files Denton Liu
2019-08-26  0:42       ` Eric Sunshine
2019-08-24  8:27     ` [PATCH 09/13] t4014: use test_line_count() where possible Denton Liu
2019-08-24  8:27     ` [PATCH 10/13] t4014: remove confusing pipe in check_threading() Denton Liu
2019-08-24  8:27     ` [PATCH 11/13] t4014: stop losing return codes of git commands Denton Liu
2019-08-24  8:27     ` [PATCH 12/13] Doc: add more detail for git-format-patch Denton Liu
2019-08-26 15:20       ` Junio C Hamano
2019-08-26 16:07         ` Junio C Hamano
2019-08-24  8:27     ` [PATCH 13/13] config/format.txt: specify default value of format.coverLetter Denton Liu
2019-08-24  8:28     ` [PATCH 00/13] format-patch: clean up tests and documentation Denton Liu
2019-08-26 15:21       ` Junio C Hamano
2019-08-27  4:04     ` [PATCH v2 " Denton Liu
2019-08-27  4:04       ` [PATCH v2 01/13] t4014: drop unnecessary blank lines from test cases Denton Liu
2019-08-27  4:04       ` [PATCH v2 02/13] t4014: s/expected/expect/ Denton Liu
2019-08-27  4:04       ` [PATCH v2 03/13] t4014: move closing sq onto its own line Denton Liu
2019-08-27  4:04       ` [PATCH v2 04/13] t4014: use sq for test case names Denton Liu
2019-08-27  4:05       ` [PATCH v2 05/13] t4014: remove spaces after redirect operators Denton Liu
2019-08-27  4:05       ` [PATCH v2 06/13] t4014: use indentable here-docs Denton Liu
2019-08-27  4:05       ` [PATCH v2 07/13] t4014: drop redirections to /dev/null Denton Liu
2019-08-27  4:05       ` [PATCH v2 08/13] t4014: let sed open its own files Denton Liu
2019-08-27  4:05       ` [PATCH v2 09/13] t4014: use test_line_count() where possible Denton Liu
2019-08-27  4:05       ` [PATCH v2 10/13] t4014: remove confusing pipe in check_threading() Denton Liu
2019-08-27  4:05       ` [PATCH v2 11/13] t4014: stop losing return codes of git commands Denton Liu
2019-08-27  4:05       ` [PATCH v2 12/13] Doc: add more detail for git-format-patch Denton Liu
2019-08-27  4:05       ` [PATCH v2 13/13] config/format.txt: specify default value of format.coverLetter Denton Liu
2019-09-04 11:21       ` [PATCH v2 00/13] format-patch: clean up tests and documentation Denton Liu
2019-09-05 19:56         ` Junio C Hamano
2019-09-05 21:40           ` Denton Liu
2019-10-11 19:12   ` [PATCH v4 0/3] format-patch: learn --cover-from-description option Denton Liu
2019-10-11 19:12     ` [PATCH v4 1/3] format-patch: remove erroneous and condition Denton Liu
2019-10-11 19:12     ` [PATCH v4 2/3] format-patch: use enum variables Denton Liu
2019-10-12  2:16       ` Junio C Hamano
2019-10-11 19:12     ` [PATCH v4 3/3] format-patch: teach --cover-from-description option Denton Liu
2019-10-12  2:36       ` Junio C Hamano
2019-10-11 19:23     ` [PATCH v4 4/3] fixup! " Denton Liu
2019-10-12  4:18     ` [PATCH v4 0/3] format-patch: learn " Junio C Hamano
2019-10-14 20:46     ` [PATCH v5 " Denton Liu
2019-10-14 20:46       ` [PATCH v5 1/3] format-patch: change erroneous and condition Denton Liu
2019-10-15  2:16         ` Junio C Hamano
2019-10-15  3:45           ` Denton Liu
2019-10-14 20:47       ` [PATCH v5 2/3] format-patch: use enum variables Denton Liu
2019-10-14 20:47       ` [PATCH v5 3/3] format-patch: teach --cover-from-description option Denton Liu
2019-10-15  2:25         ` Junio C Hamano

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=cover.1566285151.git.liu.denton@gmail.com \
    --to=liu.denton@gmail.com \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=sunshine@sunshineco.com \
    /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.