Re: [PATCH v6 1/7] pull: rename pull.rebase to pull.mode

Richard Hansen wrote:

On 2014-05-01 20:00, Felipe Contreras wrote:

quoted

Also 'branch.<name>.rebase' to 'branch.<name>.pullmode'.

This way we can add more modes and the default can be something else,
namely it can be set to merge-ff-only, so eventually we can reject
non-fast-forward merges by default.

The old configurations still work, but get deprecated.

s/get/are/

quoted

Signed-off-by: Felipe Contreras <redacted>
---
 Documentation/config.txt   | 39 ++++++++++++++++++++++-----------------
 Documentation/git-pull.txt |  2 +-
 branch.c                   |  4 ++--
 builtin/remote.c           | 14 ++++++++++++--
 git-pull.sh                | 31 +++++++++++++++++++++++++++++--
 t/t3200-branch.sh          | 40 ++++++++++++++++++++--------------------
 t/t5601-clone.sh           |  4 ++--
 7 files changed, 88 insertions(+), 46 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index c26a7c8..c028aeb 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt

@@ -708,7 +708,7 @@ branch.autosetupmerge::
 branch.autosetuprebase::
 	When a new branch is created with 'git branch' or 'git checkout'
 	that tracks another branch, this variable tells Git to set
-	up pull to rebase instead of merge (see "branch.<name>.rebase").
+	up pull to rebase instead of merge (see "branch.<name>.pullmode").
 	When `never`, rebase is never automatically set to true.
 	When `local`, rebase is set to true for tracked branches of
 	other local branches.

Should branch.autosetuprebase be replaced with a new
branch.autosetupmode setting?

Maybe. But if so, I think that should be done in another series.
Otherwise we'll never have a chance to change anything.

quoted

@@ -764,15 +764,17 @@ branch.<name>.mergeoptions::
 	option values containing whitespace characters are currently not
 	supported.
 
-branch.<name>.rebase::
-	When true, rebase the branch <name> on top of the fetched branch,
-	instead of merging the default branch from the default remote when
-	"git pull" is run. See "pull.rebase" for doing this in a non
-	branch-specific manner.
+branch.<name>.pullmode::
+	When "git pull" is run, this determines if it would either merge or
+	rebase the fetched branch.

To me this sentence implies that 'rebase' would rebase the fetched
branch onto HEAD, when it's actually the other way around.

Right.

This actually interesting mode of thinking:

a) git pull --rebase

We want to rebase HEAD onto @{upstream}.

b) git pull --merge

We want to merge HEAD into @{upstream}. (Why are we doing the opposite?)

c) git pull --rebase github john

We weant to rebase github/john onto HEAD. (We are doing the opposite?)

d) git pull --merge github john

We want to merge github/john into HEAD.

quoted

                                 The possible values are 'merge',
+	'rebase', and 'rebase-preserve'.

While the name 'merge' is mostly self-explanatory, I think it needs
further clarification:  Does 'merge' imply --no-ff?  Or --ff?  Or the
value of merge.ff?

'pull.mode=merge' will do the same as `git merge`, I don't see where or
how it can be explained more clearly.

Which side will be the first parent?

The same as things currently are: the pulled branch into the current
branch (current branch is first parent).

We should probably change this, but that's out of scope of this series,
and hasn't been decided yet.

quoted

                                       See "pull.mode" for doing this in a
+	non branch-specific manner.

I find this sentence to be a bit unclear and would prefer something
like:  "Defaults to the value of pull.mode."

Hmm, might make sense.

quoted

 +
-	When preserve, also pass `--preserve-merges` along to 'git rebase'
-	so that locally committed merge commits will not be flattened
-	by running 'git pull'.
+	When 'rebase-preserve', also pass `--preserve-merges` along to
+	'git rebase' so that locally committed merge commits will not be
+	flattened by running 'git pull'.
++
+	It was named 'branch.<name>.rebase' but that is deprecated now.

To me this sentence implies that .rebase was simply renamed to .pullmode
with no other changes.  I'd prefer something like this:

branch.<name>.rebase::
    Deprecated in favor of branch.<name>.pullmode.

(Same goes for pull.rebase.)

Right.

quoted

 +
 *NOTE*: this is a possibly dangerous operation; do *not* use
 it unless you understand the implications (see linkgit:git-rebase[1]

@@ -1881,15 +1883,18 @@ pretty.<name>::
 	Note that an alias with the same name as a built-in format
 	will be silently ignored.
 
-pull.rebase::
-	When true, rebase branches on top of the fetched branch, instead
-	of merging the default branch from the default remote when "git
-	pull" is run. See "branch.<name>.rebase" for setting this on a
-	per-branch basis.
+pull.mode::
+	When "git pull" is run, this determines if it would either merge or
+	rebase the fetched branch. The possible values are 'merge',
+	'rebase', and 'rebase-preserve'. See "branch.<name>.pullmode" for doing
+	this in a non branch-specific manner.
++
+	When 'rebase-preserve', also pass `--preserve-merges` along to
+	'git rebase' so that locally committed merge commits will not be
+	flattened by running 'git pull'.
++
 +
-	When preserve, also pass `--preserve-merges` along to 'git rebase'
-	so that locally committed merge commits will not be flattened
-	by running 'git pull'.

The default value should be documented.  Also, rather than copy+paste
the description from branch.<name>.pullmode, I'd prefer a brief
reference.  For example:

pull.mode::
    See branch.<name>.pullmode.  Defaults to 'merge'.

I'd say pull.mode is the important one.

-- 
Felipe Contreras