Hi, On Wed, 8 Apr 2009, Peter Hutterer wrote: > 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! > > > + 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... :-) > > > + 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html