All of lore.kernel.org
 help / color / mirror / Atom feed
From: Elijah Newren <newren@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Git Mailing List <git@vger.kernel.org>,
	Ulrich Windl <Ulrich.Windl@rz.uni-regensburg.de>
Subject: Re: [PATCH v2] merge-options.txt: correct wording of --no-commit option
Date: Tue, 19 Feb 2019 14:31:50 -0800	[thread overview]
Message-ID: <CABPp-BGGujXxdmdv0P+TwHwwKaK2maA6rJ7=WpiJYq7ZZivkpw@mail.gmail.com> (raw)
In-Reply-To: <xmqqk1hv1sms.fsf@gitster-ct.c.googlers.com>

On Tue, Feb 19, 2019 at 11:32 AM Junio C Hamano <gitster@pobox.com> wrote:
> Elijah Newren <newren@gmail.com> writes:
>
> > +With --no-commit perform the merge and stop just before creating
> > +a merge commit, to give the user a chance to inspect and further
> > +tweak the merge result before committing.
> > ++
> > +Note that fast-forward updates do not need to create a merge
> > +commit and therefore there is no way to stop those merges with
> > +--no-commit.  Thus, if you want to ensure your branch is not
> > +changed or updated by the merge command, use --no-ff with
> > +--no-commit.
>
> While the above is an improvement (so I'll queue it on 'pu' not to
> lose sight of it), I find the use of "do not need to" above somewhat
> misleading.  It solicits a reaction "ok, we know it does not need
> to, but it could prepare to create one to allow us to further muck
> with it, no?".
>
> IOW, a fast-forward by definition does not create a merge by itself,
> so there is nowhere to stop during a creation of a merge.  So at
> least:
>
>         s/do not need to/do not/

Yes, I agree that's a good change.  I'll wait a few days for other
feedback and resend with that and any other changes.

> It also may be a good idea to consider detecting this case and be a
> bit more helpful, perhaps with end-user experience looking like...
>
>   $ git checkout master^0
>   $ git merge --no-commit next
>   Updating 0d0ac3826a..ee538a81fe
>   Fast-forward
>     ...diffstat follows here...
>   hint: merge completed without creating a commit.
>   hint: if you wanted to prepare for a manually tweaked merge,
>   hint: do "git reset --keep ORIG_HEAD" followed by
>   hint: "git merge --no-ff --no-commit next".
>
> or even
>
>   $ git checkout master^0
>   $ git merge --no-commit next
>   warning: defaulting to --no-ff, given a --no-commit request
>   Automatic merge went well; stopped before committing as requested
>   hint: if you'd rather have a fast-forward without creating a commit,
>   hint: do "git reset --keep next" now.

Good points.  I thought of this last one before sending, though
without pre- and post- warnings/hints; without such text it definitely
seemed too magical and possibly leading to unexpected surprises in a
different direction, so I dismissed it without further thought.  But
the warnings/hints help.

> I do not have a strong preference among three (the third option
> being not doing anything), but if pressed, I'd say that the last one
> might be the most user-friendly, even though it feels a bit too
> magical and trying to be smarter than its own good.

I also lack a strong preference.  Maybe mark it #leftoverbits for
someone that does?

> In any case, the hint for the "recovery" procedure needs to be
> carefully written.

Yes.

  reply	other threads:[~2019-02-19 22:32 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-22 20:55 Q: What happened to "--no-commit" merges? Ulrich Windl
2019-01-22 21:29 ` Elijah Newren
2019-01-23  7:04   ` Antw: " Ulrich Windl
2019-02-18 18:41     ` Elijah Newren
     [not found]       ` <0A3130DD0200005B824A10E1@gwsmtp1.uni-regensburg.de>
2019-02-19  7:03         ` Antw: Antw: Ulrich Windl
2019-02-19 17:07           ` [PATCH v2] merge-options.txt: correct wording of --no-commit option Elijah Newren
2019-02-19 19:32             ` Junio C Hamano
2019-02-19 22:31               ` Elijah Newren [this message]
2019-02-19 22:38                 ` Junio C Hamano
     [not found]               ` <07106FB4020000CD824A10E1@gwsmtp1.uni-regensburg.de>
     [not found]                 ` <AC7679FC02000011B9FD70CF@gwsmtp1.uni-regensburg.de>
2019-02-20  7:29                   ` Antw: " Ulrich Windl
2019-02-21 17:50             ` [PATCH v3] " Elijah Newren

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

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

  git send-email \
    --in-reply-to='CABPp-BGGujXxdmdv0P+TwHwwKaK2maA6rJ7=WpiJYq7ZZivkpw@mail.gmail.com' \
    --to=newren@gmail.com \
    --cc=Ulrich.Windl@rz.uni-regensburg.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.