Re: [PATCH v2] submodule: Fix documentation of update subcommand

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Feb 19 2015, Junio C Hamano wrote:
> Michal Sojka <sojkam1@xxxxxxxxxxx> 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.

> 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.

>> 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.

>> +	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

>>  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 :)

>>  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
--
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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]