Re: [PATCH v6 1/7] pull: rename pull.rebase to pull.mode
- From
- Richard Hansen <rhansen@bbn.com>
- Date
- May 2, 2014, 20:45 UTC
- Message-ID
- <53640403.30600@bbn.com>
- In-Reply-To
- <1398988808-29678-2-git-send-email-felipe.contreras@gmail.com>
On 2014-05-01 20:00, Felipe Contreras wrote:
Show 7 quoted lines
> 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/
Show 25 quoted lines
> > Signed-off-by: Felipe Contreras <felipe.contreras@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?
Show 12 quoted lines
> @@ -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."
Show 9 quoted lines
> + > - 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.)
Show 26 quoted lines
> + > *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'.-Richard