Re: [PATCH] submodule: Improve documentation of update subcommand

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 23, 2015, 20:13 UTC
Message-ID: <xmqqvbiss7xb.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1424698360-10952-1-git-send-email-sojkam1@fel.cvut.cz>

Michal Sojka <sojkam1@fel.cvut.cz> writes:

> The documentation of 'git submodule update' has several problems:

Thanks, this round looks much better.

Show 14 quoted lines

> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index ae6791d..fb2ae37 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -2411,12 +2411,17 @@ 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.
> +

The sentence "edit them to override" talks about "other values", which in the original wanted to cover not just "path" but "update" as well. By splitting 'update' into its own entry, "edit them to override" is lost from 'update'.

But stepping back a bit, "edit them to override" applies to all configuration variables. The user edits the configuration file to customize things. I wonder if we even need to say this for .path and url in the first place?

    Note: not a request to remove it because I hinted so, but a
    request for comments and discussion, as I do not have a firm
    opinion.

Show 10 quoted lines

>  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. This variable
> +	is populated by `git submodule init` from the
> +	linkgit:gitmodules[5] file. See description of 'update'
> +	command in linkgit:git-submodule[1].

Show 23 quoted lines

> diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
> index 8e6af65..067d616 100644
> --- a/Documentation/git-submodule.txt
> +++ b/Documentation/git-submodule.txt
> @@ -154,27 +154,51 @@ 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. The "updating" can be done in several ways depending
> +on command line options and the value of `submodule.<name>.update`
> +configuration variable. Supported update methods are:

If you read the description of "--remote" (sorry, I didn't notice it until I formatted the result of this patch and tried to read the whole thing), we already use "update procedure" to mean these modes of updates collectively. Either use "update procedures" here (and everywhere else in this patch where it is called "update method"), or adjust the existing "update procedure" to "update method". Either way is fine, but because "update procedure" is not wrong per-se, I think it would be better to use that phrasing that may already be familiar with the "git submodule" users.