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

2014-05-02 Thread W. Trevor King
On Thu, May 01, 2014 at 07:00:02PM -0500, Felipe Contreras wrote:
 Also 'branch.name.rebase' to 'branch.name.pullmode'.

Perhaps this has already been hashed out in a previous version of this
series, but we may want to use pull.update and branch.name.update to
match the existing submodule.name.update.  Both settings are
selecting the default integration style between HEAD and some other
reference (pull's remote branch, the gitlinked commit, or the
submodule's --remote branch).

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy


signature.asc
Description: OpenPGP digital signature


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

2014-05-02 Thread Richard Hansen
On 2014-05-01 20:00, Felipe Contreras wrote:
 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/

 
 Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
 ---
  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?

 @@ -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.

  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?  Which side will be the first parent?

Similarly, 'rebase' could use some clarification:
  * the local branch is rebased onto the pulled branch, not the other
way around
  * it doesn't simply do 'git rebase FETCH_HEAD' -- it also walks the
reflog of the upstream ref until it finds an ancestor of the local
branch

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.

  +
 - 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.)

  +
  *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 

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

2014-05-02 Thread Felipe Contreras
Richard Hansen wrote:
 On 2014-05-01 20:00, Felipe Contreras wrote:
  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/
 
  
  Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
  ---
   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.

  @@ -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.

   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.

 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.

   +
  -   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.

   +
   *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 

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

2014-05-02 Thread Richard Hansen
On 2014-05-02 17:12, Felipe Contreras wrote:
 Richard Hansen wrote:
 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.

Sure.

  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.

How about:

branch.name.pullmode::
Determines how 'git pull' integrates the fetched branch into
branch name.
Defaults to the value of `pull.mode`.
Supported values:
+
--
`merge`:::
Merge the fetched branch into name.
See also `merge.ff`.
`rebase`:::
Find the point at which name forked from the fetched
branch (see the `--fork-point` option of
linkgit:git-merge-base[1]), then rebase the commits
between the fork point and branch name onto the
fetched branch.
`rebase-preserve`:::
Like `rebase` but passes `--preserve-merges` to 'git
rebase'.
--
+
*NOTE*: `rebase` and `rebase-preserve` are potentially dangerous; do
*not* use them unless you understand the implications (see
linkgit:git-rebase[1] for details).

pull.mode::
See `branch.name.pullmode`.  Defaults to `merge`.

 
 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).

This was a rhetorical question -- I was trying to point out that the
current behavior should be documented.

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

Agreed.  If this series is merged, a future series could add a
'merge-there' pull mode.

 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.

Either way works for me.  How about this:

branch.name.pullmode::
Overrides the value of `pull.mode` for branch name.

pull.mode::
Determines how 'git pull' integrates the fetched branch into
the current branch.
Supported values:
+
--
`merge`:::
(default)
Merge the fetched branch into the current branch.
See also `merge.ff`.
`rebase`:::
Find the point at which the current branch forked from
the fetched branch (see the `--fork-point` option of
linkgit:git-merge-base[1]), then rebase the commits
between the fork point and the current branch onto the
fetched branch.
`rebase-preserve`:::
Like `rebase` but passes `--preserve-merges` to 'git
rebase'.
--
+
*NOTE*: `rebase` and `rebase-preserve` are potentially dangerous; do
*not* use them unless you understand the implications (see
linkgit:git-rebase[1] for details).

-Richard
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html