All of lore.kernel.org
 help / color / mirror / Atom feed
From: Linus Arver <linusa@google.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Dragan Simic <dsimic@manjaro.org>, git@vger.kernel.org
Subject: Re: [PATCH] doc: pull: improve rebase=false documentation
Date: Fri, 15 Sep 2023 14:12:00 -0700	[thread overview]
Message-ID: <owlya5tnjg4f.fsf@fine.c.googlers.com> (raw)
In-Reply-To: <xmqqh6nvfi2p.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> writes:

> Linus Arver <linusa@google.com> writes:
>
>> Aside: interestingly, there appears to be a "--no-rebase" option that
>> means "--rebase=false" (see cd67e4d46b (Teach 'git pull' about --rebase,
>> 2007-11-28)):
>>
>>        --no-rebase
>>            This is shorthand for --rebase=false.
>> ...
>> How about adding something like this instead as the very first paragraph
>> for this flag?
>>
>>     Supplying "--rebase" defaults to "--rebase=true". Running git-pull
>>     without arguments implies "--rebase=false", unless relevant
>>     configuration variables have been set otherwise.
>
> Phrase nit.
>
> 	$ git pull origin
>
> does run the command with arguments.

Ah, good catch.

> What you mean is "running
> git-pull without any --rebase arguments implies --no-rebase",

Right (modulo your "--rebase arguments" -> "--rebase option" correction
in your follow-up email).

> but
> that is saying "not giving --rebase=<any> and not giving --rebase
> means not rebasing", which makes my head spin.

Me too.

> "--no-rebase" as a command line option does have use to defeat
> configured pull.rebase that is not set to "false"

Yes, I noticed this too after digging around a bit more after I sent my
message. Thanks for clarifying for the record.

> and allowing
> "pull.rebase" to be set to "false" does have use to defeat settings
> for the same variable made by lower-precedence configuration file.

Indeed! I did not think of this. IOW, Git can be configured in
multiple places (the "pull.rebase" setting in ~/.gitconfig can be
overridden by the same config in ~/myrepo/.git/config).

> "--rebase=false" does not have any reason to exist, except for
> making the repertoire of "--rebase=<kind>" to be complete.

Agreed. In a sense, the docs for "--rebase=false" should say that it is
a synonym for "--no-rebase" (because "--no-rebase" came first,
historically), and not the other way around (that "--no-rebase" is
shorthand for "--rebase=false").

> So, I am still not sure if saying "'git pull' (no other arguments
> and no configuration) is equivalent to 'git pull --rebase=false'"
> adds much value.

Fair point. That is, there are so many gotchas and "edge case"-like
behaviors to consider here, so the statement "'git pull' (no other arguments
and no configuration) is equivalent to 'git pull --rebase=false'" is an
oversimplification that can be misleading. I agree.

> If --no-rebase and --rebase=false are explained in terms of why
> these options that specify such an unnatural action (after all, you
> say "do this" or "do it this way", but do not usually have to say
> "do not do it that way") need to exist.
>
> If I were writing this patch, I would rearrange the existing text
> like so:
>
>  * Update the description of "--no-rebase" *NOT* to depend on
>    --rebase=false.  Instead move it higher and say
>
>    - The default for "git pull" is to "merge" the other history into
>      your history, but optionally you can "rebase" your history on
>      top of the other history.
>
>    - There are configuration variables (pull.rebase and
>      branch.<name>.rebase) that trigger the optional behaviour, and
>      when you set it, your "git pull" would "rebase".
>
>    - The "--no-rebase" option is to defeat such configuration to
>      tell the command to "merge" for this particular invocation.
>
>  * Update the description of "--rebase=<kind>" and move the
>    paragraph that begins with "When false" to the end, something
>    like:
>
>    - `--rebase` alone is equivalent to `--rebase=true`.
>    - When set to 'merges'...
>    - When set to 'interactive'...
>    - See `pull.rebase`, ..., if you want to make `git pull` always
>      rebase your history on top of theirs, instead of merging their
>      history to yours.
>    - `--rebase=false` is synonym to `--no-rebase`.

I think this captures the finer details while still preserving the
spirit of Dragan's original patch, so SGTM.

@Dragan if you are OK with doing the (much more substantial) change as
suggested, please do. Thanks!

  parent reply	other threads:[~2023-09-15 21:13 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-14  3:25 [PATCH] doc: pull: improve rebase=false documentation Dragan Simic
2023-09-14 19:00 ` Junio C Hamano
2023-09-14 23:57   ` Linus Arver
2023-09-15  5:59     ` Dragan Simic
2023-09-15 17:43     ` Junio C Hamano
2023-09-15 18:00       ` Junio C Hamano
2023-09-15 21:12       ` Linus Arver [this message]
2023-09-16  2:18         ` Dragan Simic

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=owlya5tnjg4f.fsf@fine.c.googlers.com \
    --to=linusa@google.com \
    --cc=dsimic@manjaro.org \
    --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.