Re: [PATCH 3/5] Update documentations for git-log to include the new --merges option and also its corresponding config option.

[Eric Sunshine <sunshine@sunshineco.com>]

On Sun, Mar 22, 2015 at 2:28 PM, Koosha Khajehmoogahi <koosha@posteo.de> wrote:
> Subject: Update documentations for git-log to include the new
> --merges option and also its corresponding config option.

The Subject: should be a short summary of the change; ideally less
than 70 or 72 characters. The rest of the commit message can flesh out
the description if necessary. The subject on this patch is far too
long. Also, drop capitalization, mention the area you're touching, and
drop the final period. You might say instead:

    Documentation: mention git-log --merges= and log.merges

In this case, it's probably not necessary to write anything else in
the commit message. The summary says enough, despite its conciseness.

More below.

> Signed-off-by: Koosha Khajehmoogahi <koosha@posteo.de>
> ---
> diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
> index 1f7bc67..506125a 100644
> --- a/Documentation/git-log.txt
> +++ b/Documentation/git-log.txt
> @@ -190,6 +190,9 @@ log.showroot::
>         `git log -p` output would be shown without a diff attached.
>         The default is `true`.
>
> +log.merges::
> +    Default for `--merges` option. Defaults to `show`.

To disambiguate this from the --merges option, you should probably
spell it out explicitly as `--merges=` rather than just `--merges`.

>  mailmap.*::
>         See linkgit:git-shortlog[1].
>
> diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
> index 4ed8587..398e657 100644
> --- a/Documentation/rev-list-options.txt
> +++ b/Documentation/rev-list-options.txt
> @@ -99,6 +99,12 @@ if it is part of the log message.
>  --merges::
>         Print only merge commits. This is exactly the same as `--min-parents=2`.
>
> +--merges=show|hide|only::
> +       If show is specified, merge commits will be shown in conjunction with
> +       other commits. If hide is specified, it will work like `--no-merges`.
> +       If only is specified, it will work like `--merges`. The default option
> +       is show.

By "The default option is show", do you mean when --merges= is not
specified? Perhaps it would be better to phrase it as:

    Default is `show` if `--merges=` is not specified.

> +
>  --no-merges::
>         Do not print commits with more than one parent. This is
>         exactly the same as `--max-parents=1`.

It might make more sense to rewrite the --merges and --no-merges
options in terms of the new --merges= option. For instance, something
like this:

     --merges::
        Shorthand for `--merges=only`.

    --merges={show|hide|only}::
        `show`: show merge and non-merge commits

        `hide`: omit merge commits; same as `--max-parents=1`

        `only`: show only merge commits; same as `--min-parents=2`

        Default is `show` if `--merges=` is not specified.

     --no-merges::
        Shorthand for `--merges=hide`.

Alternately, --merges and --merges= could be combined like this
(taking inspiration from --decorate[=]):

    --merges[=show|hide|only]::

But then you need additional explanation about what --merges means
without the '=' and following keyword.

> --
> 2.3.3.263.g095251d.dirty