Re: [RFC/PATCH] git-submodule: add support for --rebase.
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Apr 8, 2009, 10:44 UTC
- Message-ID
- <alpine.DEB.1.00.0904081241460.9157@intel-tinevez-2-302>
- In-Reply-To
- <20090408042212.GA4702@dingo.bne.redhat.com>
Hi,
On Wed, 8 Apr 2009, Peter Hutterer wrote:
Show 22 quoted lines
> On Tue, Apr 07, 2009 at 02:38:37PM +0200, Johannes Schindelin wrote: > > > Peter wrote originally: > > > > > diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt > > > index 3b8df44..117ad3d 100644 > > > --- a/Documentation/git-submodule.txt > > > +++ b/Documentation/git-submodule.txt > > > @@ -177,6 +178,12 @@ OPTIONS > > > This option is only valid for the update command. > > > Don't fetch new objects from the remote site. > > > > > > +--rebase:: > > > + This option is only valid for the update command. > > > > This is unnecessary, it was mentioned in the synopsis. > > Correct, but in the same file the options --cached, --no-fetch, and > --summary-limit do state that they are only valid for the > update/status/summary commands, respectively. For consistency, we should > either add this sentence to --rebase, or remove them from the others. I > personally prefer having it there, just to make it more obvious.
Fair enough!
Show 14 quoted lines
> > > + Forward-port local commits to the index of the containing repository. > > > > This is a bit misleading/unclear. I'd rather have it read like this: > > > > Instead of detaching the HEAD to the revision committed in the > > superproject, rebase the current branch onto that revision. > > Hehe. I had something like this before and then decided to copy/paste > the line from the git-rebase man page :) > > Changed to: "Rebase the current branch onto the index of the containing > repository instead of detaching the HEAD." (I vaguely remember the rule > that sentences are easier to understand if you have "blah instead of > foo" rather than "instead of foo, blah")
> The phrase "index of the containing repository" is commonly used in this > man page, so I think it's best to stick with it. That better now?
Hmm.
You can really rebase only onto a commit. And the index is not a commit, so I do not like the wording (not even in the rebase man page).
But let's see what other reviewers say... :-)
Show 17 quoted lines
> > > + If a a merge failure prevents this process, you will have to resolve > > > + these failures with linkgit:git-rebase[1]. > > > + > > > <path>...:: > > > Paths to submodule(s). When specified this will restrict the command > > > to only operate on the submodules found at the specified paths. > > > diff --git a/git-submodule.sh b/git-submodule.sh > > > index 7c2e060..6180bf4 100755 > > > --- a/git-submodule.sh > > > +++ b/git-submodule.sh > > > > You might want to error out when --rebase was passed with another > > command than "update". > > cmd_init(), cmd_summary(), etc. have catch-all rules for unknown options > to display the usage, so this is covered. (e.g. line 439, > git-submodule.sh)
Oh, okay!
Other than the wording, I am completely happy with this patch.
Ciao, Dscho