All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Elijah Newren <newren@gmail.com>
Cc: Elijah Newren via GitGitGadget <gitgitgadget@gmail.com>,
	Git Mailing List <git@vger.kernel.org>,
	Alex Henrie <alexhenrie24@gmail.com>,
	Phillip Wood <phillip.wood123@gmail.com>,
	Son Luong Ngoc <sluongng@gmail.com>
Subject: Re: [PATCH 3/5] pull: handle conflicting rebase/merge options via last option wins
Date: Thu, 15 Jul 2021 12:58:39 -0700	[thread overview]
Message-ID: <xmqqpmvjs61c.fsf@gitster.g> (raw)
In-Reply-To: <CABPp-BExWMSFr7CQskjKVhr5fiWCnxoaN_RaJ2Yir+36aiyBjQ@mail.gmail.com> (Elijah Newren's message of "Thu, 15 Jul 2021 12:04:38 -0700")

Elijah Newren <newren@gmail.com> writes:

> Let me ask two questions:
>
> 1. When is it beneficial for users to set both pull.ff and pull.rebase?
> 2. Is it harmful to users for us to allow both to be set when we will
> just ignore one?
>
> I believe the answer to (1) is "never", and the answer to (2) is "yes".

I agree (1) never gives you anything, even though it does not hurt,
and (2) is "meh".

> For the second question in particular, I can think of two example cases:
>
> 2a) Users start with pull.ff=only, perhaps suggested by someone else
> and left in their config for a long time.  When users hit a case that
> can't fast-forward and they either ask for help or find a post on
> stack overflow that suggests setting pull.rebase=true, they do so and
> then get no warning that the setting they just added is being ignored.

Well, overriding "only fast-forward is allowed" with "instead of
merge, you can rebase" is a nonsense suggestion in the first place,
no?  Why does Git suddenly become responsible for such a misguided
suggestion?

> 2b) Perhaps users start with pull.rebase=true (suggested by a
> colleague and forgot about it as they are more of a tester than a
> developer and thus usually only see fast-forwards).  Then at some
> point they need to function as an integrator, and they read the docs
> and determine that pull.ff=false should do what they want to create
> merge commits.

Again, "I want to pee in the snow" is not what you need to act as an
integrator.  I do not see how relevant this example is, either.  You
are just reacting to a wrong suggestion.

> But then they get shocked that they've rebased public
> commits (and perhaps also pushed them out) when they wanted merges.
> Our docs have pretty clearly stated that pull.ff=false and --no-ff
> create merges.

That is something we need to and can fix.  The "pee in the snow
commit can be created by passing --no-ff" was written back when the
designed audiences of "pull" were primarily those who merge (think
of "pull --rebase" as afterthought).  IOW, to the minds of those who
originally wrote --no-ff feature (and its doc), "pull --rebase" was
not in the picture.


  reply	other threads:[~2021-07-15 19:59 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-15  2:40 [PATCH 0/5] Handle conflicting pull options Elijah Newren via GitGitGadget
2021-07-15  2:40 ` [PATCH 1/5] pull: move definitions of parse_config_rebase and parse_opt_rebase Elijah Newren via GitGitGadget
2021-07-15  2:40 ` [PATCH 2/5] pull: convert OPT_PASSTHRU for fast-forward options to OPT_CALLBACK Elijah Newren via GitGitGadget
2021-07-15  2:40 ` [PATCH 3/5] pull: handle conflicting rebase/merge options via last option wins Elijah Newren via GitGitGadget
2021-07-15  4:59   ` Eric Sunshine
2021-07-15 17:13     ` Elijah Newren
2021-07-15  9:44   ` Felipe Contreras
2021-07-15 17:33   ` Junio C Hamano
2021-07-15 17:46     ` Felipe Contreras
2021-07-15 19:04     ` Elijah Newren
2021-07-15 19:58       ` Junio C Hamano [this message]
2021-07-15 20:40         ` Elijah Newren
2021-07-15 21:12           ` Junio C Hamano
2021-07-16 18:39             ` Elijah Newren
2021-07-16 21:18               ` Junio C Hamano
2021-07-16 21:56                 ` Felipe Contreras
2021-07-15 20:17       ` Junio C Hamano
2021-07-15 20:38         ` Elijah Newren
2021-07-15  2:40 ` [PATCH 4/5] pull: abort if --ff-only is given and fast-forwarding is impossible Alex Henrie via GitGitGadget
2021-07-15  2:40 ` [PATCH 5/5] pull: abort by default when fast-forwarding is not possible Elijah Newren via GitGitGadget
2021-07-15  5:18   ` Eric Sunshine
2021-07-15 16:56     ` Elijah Newren
2021-07-15  9:48   ` Felipe Contreras
2021-07-16  9:32   ` Ævar Arnfjörð Bjarmason
2021-07-16 18:13     ` Felipe Contreras
2021-07-15  9:37 ` [PATCH 0/5] Handle conflicting pull options Felipe Contreras

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=xmqqpmvjs61c.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=alexhenrie24@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=newren@gmail.com \
    --cc=phillip.wood123@gmail.com \
    --cc=sluongng@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.
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.