On Wed, Jul 14, 2021 at 10:41 PM Elijah Newren via GitGitGadget
<gitgitgadget@xxxxxxxxx> wrote:
> We have for some time shown a long warning when the user does not
> specify how to reconcile divergent branches with git pull. Make it an
> error now.
>
> Signed-off-by: Elijah Newren <newren@xxxxxxxxx>
Teeny tiny nits below...
> diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
> @@ -15,14 +15,16 @@ SYNOPSIS
> -Incorporates changes from a remote repository into the current
> -branch. In its default mode, `git pull` is shorthand for
> -`git fetch` followed by `git merge FETCH_HEAD`.
> -
> -More precisely, 'git pull' runs 'git fetch' with the given
> -parameters and calls 'git merge' to merge the retrieved branch
> -heads into the current branch.
> -With `--rebase`, it runs 'git rebase' instead of 'git merge'.
The original text has an odd mix of apostrophes and backticks...
> +Incorporates changes from a remote repository into the current branch.
> +If the current branch is behind the remote, then by default it will
> +fast-forward the current branch to match the remote. If the current
> +branch and the remote have diverged, the user needs to specify how to
> +reconcile the divergent branches with --no-ff or --rebase (or the
> +corresponding configuration options in pull.ff or pull.rebase).
> +
> +More precisely, 'git pull' runs 'git fetch' with the given parameters
> +and then depending on config options or command line flags, will call
> +either 'git merge' or 'git rebase' to reconcile diverging branches.
... and the revised text adds "no quotes" to the selection. These
days, we'd probably backtick all of these:
--no-ff
--rebase
pull.ff
pull.rebase
git pull
git fetch
git merge
git rebase
The rest of this document is actually pretty good about using
backticks, though there are some exceptions.
There's also an odd mix of "configuration options" and "config
options" in the revised text. Perhaps stick with "configuration
options" to be a bit more formal?
> <repository> should be the name of a remote repository as
> passed to linkgit:git-fetch[1]. <refspec> can name an
And, as an aside, we'd backtick <repository> and <refspec>, though
your patch isn't touching that, so outside the scope of this change.
> diff --git a/builtin/pull.c b/builtin/pull.c
> @@ -1074,9 +1074,8 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
> if (opt_ff) {
> if (!strcmp(opt_ff, "--ff-only"))
> die_ff_impossible();
> - } else {
> - if (rebase_unspecified && opt_verbosity >= 0)
> - show_advice_pull_non_ff();
> + } else if (rebase_unspecified) {
> + die_pull_non_ff();
> }
When reading the previous patch, I was wondering why an `if else`
wasn't used, and here I see that it does become an `if else`. I guess
you didn't want to alter Alex's original patch?
