mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Martin Ågren" <>
To: Elijah Newren <>
Cc: Junio C Hamano <>,
	Git Mailing List <>,
	Sergey Organov <>
Subject: Re: [PATCH v2] merge-options.txt: clarify meaning of various ff-related options
Date: Wed, 28 Aug 2019 20:45:27 +0200	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

Hi Elijah,

On Wed, 28 Aug 2019 at 17:52, Elijah Newren <> wrote:
> ---ff::
> -       When the merge resolves as a fast-forward, only update the branch
> -       pointer, without creating a merge commit.  This is the default
> -       behavior.
> -
> +--ff-only::
>  --no-ff::
> -       Create a merge commit even when the merge resolves as a
> -       fast-forward.  This is the default behaviour when merging an
> -       annotated (and possibly signed) tag that is not stored in
> -       its natural place in 'refs/tags/' hierarchy.
> +--ff::
> +       Whether to only allow resolving the merge as a fast forward
> +       (only updating the branch pointer to match the merged branch
> +       and not creating a merge commit), to never allow it (always
> +       creating a merge commit), or to prefer it when possible.  The
> +       default is --ff, except when merging an annotated (and

It would be great if you'd write this as `--ff` so that it got
monospaced in the rendered documentation. Same thing with `no-ff` later
in this paragraph and a few more times in the next three paragraphs that
you're adding.

> +       possibly signed) tag that is not stored in its natural place
> +       in 'refs/tags/' hierarchy (in which case --no-ff is the
> +       default).

I'd also write `refs/tags/`, but I realize that you're just inheriting
what is already here. If you'd rather punt on that, that's understood.
This whole document could need a look-over with respect to monospacing
anyway, but it seems unfortunate to introduce *new* non-monospaced
instances, especially for command-line options where it's pretty clear
how they should be handled (Documentation/CodingGuidelines, line ~610).

> ++
> +With --ff-only, resolve the merge as a fast-forward when possible
> +(when the merged branch contains the current branch in its history).
> +When not possible, refuse to merge and exit with a non-zero status.
> ++
> +With --no-ff, create a merge commit in all cases, even when the merge
> +could instead resolve as a fast-forward.
> ++
> +With --ff, resolve the merge as a fast-forward when possible.  When not
> +possible, create a merge commit.
> ---ff-only::
> -       Refuse to merge and exit with a non-zero status unless the
> -       current `HEAD` is already up to date or the merge can be
> -       resolved as a fast-forward.

I was sort of expecting these to be listed in the order "--ff, --no-ff,
--ff-only", and I see Sergey suggested the same ordering. The way your
proposed text reads does make sense though... Would it read as well
turning it over and going through the options in the other order? That's
the way it is before your patch, so you could argue "but people don't
grok that!". What your patch does well is to offer an overview before
describing each option in a bit more detail. Would that work with the
reversed direction as well (compared to this patch)? Dunno.

I wondered briefly whether it might make sense to float "The default is
`--no-ff`." to the top, but since it's really "The default ... unless
so-and-so", it would probably complicate things more than it'd help.


  reply	other threads:[~2019-08-28 18:45 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-28  0:13 [PATCH] merge-options.txt: clarify meaning of various ff-related options Elijah Newren
2019-08-28  9:05 ` Sergey Organov
2019-08-28 15:51   ` [PATCH v2] " Elijah Newren
2019-08-28 18:45     ` Martin Ågren [this message]
2019-08-28 19:15       ` Sergey Organov
2019-08-28 19:53         ` Martin Ågren
2019-08-29  9:35           ` Sergey Organov
2019-08-28 22:51         ` Elijah Newren
2019-08-29  9:15           ` Sergey Organov
2019-08-28 22:57       ` [PATCH v3] " Elijah Newren
2019-08-30 19:57         ` Junio C Hamano
2019-08-30 20:16           ` Eric Sunshine
2019-08-31  0:23             ` [PATCH v4] " Elijah Newren
2019-08-30 19:45       ` [PATCH v2] " Junio C Hamano
2019-08-30 19:48         ` Elijah Newren
2019-08-30 20:27           ` 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:

  List information:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \

* 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.
Code repositories for project(s) associated with this public inbox

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).