git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Sergey Organov <sorganov@gmail.com>
To: Elijah Newren <newren@gmail.com>
Cc: "Martin Ågren" <martin.agren@gmail.com>,
	"Junio C Hamano" <gitster@pobox.com>,
	"Git Mailing List" <git@vger.kernel.org>
Subject: Re: [PATCH v2] merge-options.txt: clarify meaning of various ff-related options
Date: Thu, 29 Aug 2019 12:15:07 +0300	[thread overview]
Message-ID: <874l20nxic.fsf@osv.gnss.ru> (raw)
In-Reply-To: <CABPp-BEsRL-JipotZ2FyrXiPcry6aSAvL8e5cbOm5jrPM63j-g@mail.gmail.com> (Elijah Newren's message of "Wed, 28 Aug 2019 15:51:14 -0700")

Elijah Newren <newren@gmail.com> writes:

> On Wed, Aug 28, 2019 at 12:15 PM Sergey Organov <sorganov@gmail.com> wrote:
>>
>> Hi,
>>
> [...]
>> Dunno if it helps, but here is what I came up with somewhere in previous
>> discussions:
>>
>> --ff::
>> --no-ff::
>> --ff-only::
>>         When the merge resolves as a fast-forward, only update the
>
> I think this loose wording (that you just took from the original) is
> problematic.  Saying that a "merge resolves as a fast-forward" seems
> to imply that there are circumstances when a fast-forward is the only
> option.  An _individual_ can decide to resolve a merge as a
> fast-forward in some circumstances, but it's certainly not the only
> choice in any circumstance.  If you want to keep this wording short,
> you could replace "resolves" with "can be resolved".
>
>>         branch pointer (without creating a merge commit).  When a fast
>
> Only update the branch pointer to what?  (Yes, I know the original
> text we were improving left this unclear, but it's worth noting.)
>
>>         forward update is not possible, create a merge commit.  This is
>>         the default behavior, unless merging an annotated (and possibly
>>         signed) tag that is not stored in its natural place in
>>         'refs/tags/' hierarchy, in which case --no-ff is assumed.
>
> Maybe it's just me, but I think it takes extra human cycles to figure
> out that this paragraph is referring just to the --ff case, and that
> users might not be able to do so until after reading the next 2-3
> sentences.  While more brief, I think it will cause people to need to
> read the description for these three options twice, removing most the
> savings from being shorter.  It'd be better if it could be re-worded
> to not need re-reads.
>
>> +
>> With --no-ff create a merge commit even when the merge could instead
>> resolve as a fast-forward.
>> +
>> With --ff-only resolve the merge as a fast-forward (never create a merge
>> commit). When fast-forward is not possible, refuse to merge and exit
>> with non-zero status.
>
> Something else I was trying to address with my patch that perhaps you
> can see a different way to tackle: Using the wording "when possible"
> is probably going to make users wonder when a fast forward is
> possible; the "can be resolved" wording tweak also makes it more
> likely they will wonder about this.  Another question they will be
> wondering about is what a fast forward is (which you partially
> explain).  Some basic knowledge of both are probably very useful in
> helping them decide which option to actually pick.  As such, I think
> trying to explain the answers to these sub-questions will assist them
> in knowing which option to use.  Simply inserting a couple phrases
> (e.g. "when the merged branch contains the current branch in its
> history", and "only update the branch pointer *to match the merged
> branch* and do not create a merge commit") may help a lot.
>
> Anyway, I'll send a v3 addressing Martin's comments; if you've got
> further suggestions for streamlining or rearranging, though, please do
> send them along.

Thanks for thorough reply!

My version was meant to show how to re-arrange the description
preserving original wording as much as possible, so your version should
be better, as it addresses other problems as well.

-- 
Sergey

  reply	other threads:[~2019-08-29  9:15 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
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 [this message]
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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=874l20nxic.fsf@osv.gnss.ru \
    --to=sorganov@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=martin.agren@gmail.com \
    --cc=newren@gmail.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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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).