[PATCH v2 0/4] more automation for cover letter generation

[Frank Terbeck <ft@bewatermyfriend.org>]

Sorry for taking this long, I was covered in work.

Junio C Hamano <gitster@pobox.com> wrote:
> Frank Terbeck <ft@bewatermyfriend.org> writes:
> > [format]
> >     coverletter = true
> >     coveronepatch = false
> 
> Nobody wants a cover letter to a single patch, so a better way
> would probably be:
> 
> 	'yes' means default behaviour, that is add cover letter for
> 	multiple-patch series, non for a single patch;
> 
> 	'no' means no cover; and
> 
> 	'always' means a probably insane "cover even a single patch".
> 
> In any case, because this new feature is way too late to be in the
> upcoming 1.6.3 release anyway, I think it is a saner approach to add a
> command line option "--cover=yes" to "cover if multiple", "--cover=always"
> to "cover even a single patch", and "--cover=no" to countermand a
> configured "format.cover" the user may have in the configuration from the
> command line.

You're right. And so is Jeff.
I'll send a followup, that will address comments of the both of you.
See below.

> 
> >     overwritecoverletter = false
> 
> I do not think it is particularly a good idea, and it is a good idea to
> have it in the configuration.
> 
>  - Why not protect the earlier patch output?  People often tweak messages
>    (both above and below the three-dash lines) in them.
> 
>  - Isn't this pretty much per invocation?
> 
> I can understand (I may not be enthused about it) a new --clobber={yes,no}
> command line option to allow/forbid clobbering the existing files, and you
> may want to add --clobber=patches to allow clobbering only the patches but
> not cover (which I do not think makes much sense, though).

This was a followup idea, which I thought was a reasonable. In the
meantime, I feel this is not needed. I'd agree that a more generic
--clobber option would make sense but frankly, protecting overwrites at
this stage is not something worth putting much effort into, IMHO.

Jeff King <peff@peff.net> wrote:
> On Sat, Apr 18, 2009 at 06:16:15PM +0200, Frank Terbeck wrote:
[...]
> Something about "coveronepatch" seems a bit hack-ish to me. Perhaps it
> should be "generate cover letter if there are more than N patches". You
> could even just overload "format.coverletter" as:
> 
>   true - always generate cover letter
>   false - never generate cover letter
>   <number> - generate if there are at least <number> patches

Yeah, a lot better.
As Junio said, noone wants cover letters for single patches.
Therefore, I made format.coverletter default to 2. Setting it to one
makes --cover-letter produce for cover letter for single patches, too.
Setting it to 0 disables cover letters, even with --cover-letter. Any
other value sets the minimum size of patch series that will trigger
--cover-letter to create a cover letter.

That can be altered from the command-line via:
    --cover-letter=always
    --cover-letter=never
    --cover-letter=<length>

In addition to that I did add format.coverauto.
Setting it to true (it defaults to false) makes format-patch behave like
the user gave the --cover-letter option - with the following exception:
if --stdout is specified, cover letters are disabled. To force cover
letters in that case too, --cover-letter needs to be specified *after*
--stdout.

Now that I think of it again, two weeks after writing this mail
originally, I guess it would be possible to drop format.coverauto
altogether and tell users to use:

  % git config --global alias.fp format-patch --cover-letter

I don't know which solution would be preferred. If it's the user-should-
use-an-alias approach, I'll create a series that gets rid of
format.coverauto changes.

[...]
> > The series is based on master and doesn't seem to break anything
> > within the test suite. It could maybe use own tests, but I must admit
> > that I didn't look too closely at how git's test suite works and where
> > to put in tests for this.
> 
> The tests below were not actually run (and you can see they are based on
> what I proposed above, not your existing patches), but they should give
> hopefully give you a headstart.

Yes, it did. Thank you.

Frank Terbeck (4):
The new options and changes:
  Add format.coverletter option
  Add format.coverauto boolean

Tests, thanks to Jeff:
  Add tests for coverauto, coverletter and --cover-letter

And finally documentation.
  Documentation for format.coverletter and --cover-letter

 Documentation/config.txt           |    9 +++++
 Documentation/git-format-patch.txt |   17 ++++++++--
 builtin-log.c                      |   32 +++++++++++++++++--
 t/t3400-rebase.sh                  |    1 +
 t/t4014-format-patch.sh            |   58 ++++++++++++++++++++++++++++++++++-
 5 files changed, 108 insertions(+), 9 deletions(-)