Re: [PATCH v8 1/4] format-patch: move cover letter summary generation

[Junio C Hamano <gitster@pobox.com>]

Mirko Faina <mroik@delayed.space> writes:

> As of now format-patch allows generation of a template cover letter for
> patch series through "--cover-letter".

"As of now" is redundant.  Our convention is to describe the state
of the codebase _before_ your changes in the present tense to set
the stage for describing the problem.

> Move shortlog summary code generation to its own function. This is done
> in preparation to other patches where we enable the user to format the
> commit list using thier own format string.

Makes sense.

>
> Signed-off-by: Mirko Faina <mroik@delayed.space>
> ---
>  builtin/log.c | 32 ++++++++++++++++++++------------
>  1 file changed, 20 insertions(+), 12 deletions(-)
>
> diff --git a/builtin/log.c b/builtin/log.c
> index 5c9a8ef363..0d12272031 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -1324,6 +1324,25 @@ static void get_notes_args(struct strvec *arg, struct rev_info *rev)
>  	}
>  }
>  
> +static void generate_shortlog_cover_letter(struct shortlog *log,
> +					   struct rev_info *rev,
> +					   struct commit **list,
> +					   int nr)
> +{
> +	shortlog_init(log);
> +	log->wrap_lines = 1;
> +	log->wrap = MAIL_DEFAULT_WRAP;
> +	log->in1 = 2;
> +	log->in2 = 4;
> +	log->file = rev->diffopt.file;
> +	log->groups = SHORTLOG_GROUP_AUTHOR;
> +	shortlog_finish_setup(log);
> +	for (int i = 0; i < nr; i++)
> +		shortlog_add_commit(log, list[i]);
> +
> +	shortlog_output(log);
> +}
> +
>  static void make_cover_letter(struct rev_info *rev, int use_separate_file,
>  			      struct commit *origin,
>  			      int nr, struct commit **list,
> @@ -1377,18 +1396,7 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,
>  	free(pp.after_subject);
>  	strbuf_release(&sb);
>  
> -	shortlog_init(&log);
> -	log.wrap_lines = 1;
> -	log.wrap = MAIL_DEFAULT_WRAP;
> -	log.in1 = 2;
> -	log.in2 = 4;
> -	log.file = rev->diffopt.file;
> -	log.groups = SHORTLOG_GROUP_AUTHOR;
> -	shortlog_finish_setup(&log);
> -	for (i = 0; i < nr; i++)
> -		shortlog_add_commit(&log, list[i]);
> -
> -	shortlog_output(&log);
> +	generate_shortlog_cover_letter(&log, rev, list, nr);
>  
>  	/* We can only do diffstat with a unique reference point */
>  	if (origin)