Re: [PATCH v2] submodule: Fix documentation of update subcommand
- From
Michal Sojka <sojkam1@fel.cvut.cz>
- Date
- Feb 19, 2015, 17:54 UTC
- Message-ID
- <87d255zt0j.fsf@steelpick.2x.cz>
- In-Reply-To
- <xmqqbnkq23a0.fsf@gitster.dls.corp.google.com>
On Thu, Feb 19 2015, Junio C Hamano wrote:
Show 10 quoted lines
> Michal Sojka <sojkam1@fel.cvut.cz> writes: > >> The documentation of 'git submodule update' has several problems: >> >> 1) It says that submodule.$name.update can be overridden by --checkout >> only if its value is `none`. > > Hmm, I do not read the existing sentence that way, though. The > "only if" above is only in your head and not in the documentation, > no?
Yes, you're right.
Show 6 quoted lines
> The way I understand it is that the explanation does not even bother > to say that it is overridable when update is set to something that > clearly corresponds to --option (e.g. 'update=rebase' is for people > too lazy to type --rebase from the command line), but because it is > unclear when it is set to 'update=none', it specifically singles out > that case.
I updated the commit message a bit.
Show 32 quoted lines
>> diff --git a/Documentation/config.txt b/Documentation/config.txt >> index ae6791d..f30cbbc 100644 >> --- a/Documentation/config.txt >> +++ b/Documentation/config.txt >> @@ -2411,12 +2411,29 @@ status.submodulesummary:: >> >> submodule.<name>.path:: >> submodule.<name>.url:: >> + The path within this project and URL for a submodule. These >> + variables are initially populated by 'git submodule init'; >> + edit them to override the URL and other values found in the >> + `.gitmodules` file. See linkgit:git-submodule[1] and >> + linkgit:gitmodules[5] for details. >> + > > OK. > >> submodule.<name>.update:: >> - The path within this project, URL, and the updating strategy >> - for a submodule. These variables are initially populated >> - by 'git submodule init'; edit them to override the >> - URL and other values found in the `.gitmodules` file. See >> - linkgit:git-submodule[1] and linkgit:gitmodules[5] for details. >> + The default updating strategy for a submodule, used by `git >> + submodule update`. This variable is populated by `git >> + submodule init` from linkgit:gitmodules[5]. >> + >> + If the value is 'checkout' (the default), the new commit >> + specified in the superproject will be checked out in the > > Have you formatted this? I _think_ this change would break the > typesetting by having an empty line there.
Right. I need to add a '+' and deindent.
Show 15 quoted lines
>> + submodule on a detached HEAD. >> + If 'rebase', the current branch of the submodule will be >> + rebased onto the commit specified in the superproject. >> + If 'merge', the commit specified in the superproject will be >> + merged into the current branch in the submodule. If 'none', >> + the submodule with name `$name` will not be updated by >> + default. >> + If the value is of form '!command', it will cause `command` to >> + be run. `command` can be any arbitrary shell command that >> + takes a single argument, namely the sha1 to update to. > > I have a feeling that it is better to leave the explanations of > these values in git-submodule.txt (i.e. where you took the above > text from) and say "see description of 'update' command in > linkgit:git-submodule[1]" here to avoid duplication.
OK
Show 23 quoted lines
>> submodule.<name>.branch:: >> The remote branch name for a submodule, used by `git submodule >> diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt >> index 8e6af65..c92908e 100644 >> --- a/Documentation/git-submodule.txt >> +++ b/Documentation/git-submodule.txt >> @@ -154,14 +154,13 @@ If `--force` is specified, the submodule's work tree will be removed even if >> it contains local modifications. >> >> update:: >> - Update the registered submodules, i.e. clone missing submodules and >> - checkout the commit specified in the index of the containing repository. >> - This will make the submodules HEAD be detached unless `--rebase` or >> - `--merge` is specified or the key `submodule.$name.update` is set to >> - `rebase`, `merge` or `none`. `none` can be overridden by specifying >> - `--checkout`. Setting the key `submodule.$name.update` to `!command` >> - will cause `command` to be run. `command` can be any arbitrary shell >> - command that takes a single argument, namely the sha1 to update to. >> + Update the registered submodules to match what the superproject >> + expects by cloning missing submodules and updating the working >> + tree of the submodules.... > > This part is better than the original.
Indeed. You wrote this in a previous email :)
Show 34 quoted lines
>> The "updating" can take various forms >> + and can be configured in .git/config by the >> + `submodule.$name.update` key or by explicitely giving one of >> + '--checkout' (the default), '--merge' or '--rebase' options. See >> + linkgit:git-config[1] for details. > > Because submodule.<name>.update is interesting only to those who run > "git submodule update", and also the command line options that > interact with the setting are only described here not in config.txt, > I think it is better to have the description of various modes here. > > And the description, if it is done here, can clarify the precedence > (i.e. command line trumps configuration) and semantics > (i.e. configuration 'update=checkout' and option --checkout are both > to trigger the same behaviour), perhaps like this: > > The updating can be done in one of three ways: > > checkout;; detaches the HEAD in the submodule at the commit > that is recorded by the superproject. This is done when > --checkout option is given, or no option is given, and > submodule.<name>.update is unset, or if it is set to > 'checkout'. > rebase;; EXPLAIN IN A SIMILAR WAY, talk about --rebase, > 'rebase', etc. > merge;; EXPLAIN IN A SIMILAR WAY, talk about --merge, > 'merge', etc. > > When no option is given and submodule.<name>.update is set > to 'none', the submodule is not updated. > > It would be awkward to talk about --option in any of the other pages > like config.txt and gitmodules.txt, but the relationship between the > options and configurations must be explained somewhere, so....
Agreed expect that there is a fourth way: !command. But this could be easily added here as well.
I'll send an updated patch in a while.
Thanks. -Michal