git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

Previous: Peter Hutterer
Message 4 of 4 in “git-submodule: add support for --rebase.”
  1. git-submodule: add support for --rebase.Peter Hutterer, Apr 7, 2009
  2. Johannes SchindelinApr 7, 2009
  3. Peter HuttererApr 8, 2009
  4. Johannes SchindelinApr 8, 2009

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.