git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH] Documentation/config: clarify the meaning of submodule.<name>.update
@ 2017-09-22 21:28 Stefan Beller
  2017-09-22 21:37 ` Jonathan Nieder
  0 siblings, 1 reply; 12+ messages in thread
From: Stefan Beller @ 2017-09-22 21:28 UTC (permalink / raw)
  To: git; +Cc: Stefan Beller

With more commands (that potentially change a submodule) paying attention
to submodules as well as the recent discussion[1] on submodule.<name>.update,
let's spell out that submodule.<name>.update is strictly to be used
for configuring the "submodule update" command and not to be obeyed
by other commands.

These other commands usually have a strict meaning of what they should
do (i.e. checkout, reset, rebase, merge) as well as have their name
overlapping with the modes possible for submodule.<name>.update.

[1] https://public-inbox.org/git/4283F0B0-BC1C-4ED1-8126-7E512D84484B@gmail.com/

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 Documentation/config.txt | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index dc4e3f58a2..b0ded777fe 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -3085,10 +3085,9 @@ submodule.<name>.url::
 	See linkgit:git-submodule[1] and linkgit:gitmodules[5] for details.
 
 submodule.<name>.update::
-	The default update procedure 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].
+	The method how a submodule is updated via 'git submodule update'.
+	It is populated by `git submodule init` from the linkgit:gitmodules[5]
+	file. See description of 'update' command in linkgit:git-submodule[1].
 
 submodule.<name>.branch::
 	The remote branch name for a submodule, used by `git submodule
-- 
2.14.0.rc0.3.g6c2e499285


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] Documentation/config: clarify the meaning of submodule.<name>.update
  2017-09-22 21:28 [PATCH] Documentation/config: clarify the meaning of submodule.<name>.update Stefan Beller
@ 2017-09-22 21:37 ` Jonathan Nieder
  2017-09-22 22:52   ` [PATCHv2] " Stefan Beller
  0 siblings, 1 reply; 12+ messages in thread
From: Jonathan Nieder @ 2017-09-22 21:37 UTC (permalink / raw)
  To: Stefan Beller; +Cc: git

Hi,

Stefan Beller wrote:

> With more commands (that potentially change a submodule) paying attention
> to submodules as well as the recent discussion[1] on submodule.<name>.update,
> let's spell out that submodule.<name>.update is strictly to be used
> for configuring the "submodule update" command and not to be obeyed
> by other commands.

Good idea, thank you.

You'll want to update Documentation/gitmodules.txt, too.

I think this can go further: it should say explicitly that commands
like "git checkout --recurse-submodules" do not pay attention to this
option.

> These other commands usually have a strict meaning of what they should
> do (i.e. checkout, reset, rebase, merge) as well as have their name
> overlapping with the modes possible for submodule.<name>.update.
>
> [1] https://public-inbox.org/git/4283F0B0-BC1C-4ED1-8126-7E512D84484B@gmail.com/

Can you summarize what this discussion concluded with so the reader
does not have to look far to understand it?

> Signed-off-by: Stefan Beller <sbeller@google.com>

Reported-by: Lars Schneider <larsxschneider@gmail.com>

> ---
>  Documentation/config.txt | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index dc4e3f58a2..b0ded777fe 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -3085,10 +3085,9 @@ submodule.<name>.url::
>  	See linkgit:git-submodule[1] and linkgit:gitmodules[5] for details.
>  
>  submodule.<name>.update::
> -	The default update procedure 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].
> +	The method how a submodule is updated via 'git submodule update'.
> +	It is populated by `git submodule init` from the linkgit:gitmodules[5]
> +	file. See description of 'update' command in linkgit:git-submodule[1].
>  

Wording nits: s/The method how/The method by which/; s/via/by/

More importantly, can this be more explicit about how it is meant to
be used?  E.g. to say

 1. This only affects "git submodule update" and doesn't affect
    commands like "git checkout --recurse-submodules".

 2. It exists for historical reasons; settings like submodule.active
    and pull.rebase are more likely to be what someone is looking for.

Thanks and hope that helps,
Jonathan

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCHv2] Documentation/config: clarify the meaning of submodule.<name>.update
  2017-09-22 21:37 ` Jonathan Nieder
@ 2017-09-22 22:52   ` Stefan Beller
  2017-09-22 22:58     ` Jonathan Nieder
  0 siblings, 1 reply; 12+ messages in thread
From: Stefan Beller @ 2017-09-22 22:52 UTC (permalink / raw)
  To: jrnieder; +Cc: git, sbeller

With more commands (that potentially change a submodule) paying attention
to submodules as well as the recent discussion[1] on
submodule.<name>.update, let's spell out that submodule.<name>.update
is strictly to be used for configuring the "submodule update" command
and not to be obeyed by other commands.

These other commands usually have a strict meaning of what they should
do (i.e. checkout, reset, rebase, merge) as well as have their name
overlapping with the modes possible for submodule.<name>.update.

[1] https://public-inbox.org/git/4283F0B0-BC1C-4ED1-8126-7E512D84484B@gmail.com/
    submodule.<name>.update was set to "none", triggering unexpected
    behavior as the submodule was thought to never be touched.
    However a newer version of Git taught 'git pull --rebase' to also
    populate and rebase submodules if they were active.
    The newer options such as submodule.active and command specific
    flags would not have triggered unexpected behavior.

Reported-by: Lars Schneider <larsxschneider@gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
 Documentation/config.txt | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

Jonathan writes:
> You'll want to update Documentation/gitmodules.txt, too.

No. /grumpycat

It should already be fine, as I read it as if it is only relevant to 
"git submodule update" there, already:

  submodule.<name>.update::
	Defines the default update procedure for the named submodule,
	i.e. how the submodule is updated by "git submodule update"
	command in the superproject. This is only used by `git
	submodule init` to initialize the configuration variable of
	the same name. Allowed values here are 'checkout', 'rebase',
	'merge' or 'none'. See description of 'update' command in
	linkgit:git-submodule[1] for their meaning. Note that the
	'!command' form is intentionally ignored here for security
	reasons.
 

diff --git a/Documentation/config.txt b/Documentation/config.txt
index dc4e3f58a2..1ac0ae6adb 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -3085,10 +3085,14 @@ submodule.<name>.url::
 	See linkgit:git-submodule[1] and linkgit:gitmodules[5] for details.
 
 submodule.<name>.update::
-	The default update procedure 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].
+	The method by which a submodule is updated by 'git submodule update',
+	which is the only affected command, others such as
+	'git checkout --recurse-submodules' are unaffected. It exists for
+	historical reasons, when 'git submodule' was the only command to
+	interact with submodules; settings like `submodule.active`
+	and `pull.rebase` are more specific. It is populated by
+	`git submodule init` from the linkgit:gitmodules[5] file.
+	See description of 'update' command in linkgit:git-submodule[1].
 
 submodule.<name>.branch::
 	The remote branch name for a submodule, used by `git submodule
-- 
2.14.0.rc0.3.g6c2e499285


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCHv2] Documentation/config: clarify the meaning of submodule.<name>.update
  2017-09-22 22:52   ` [PATCHv2] " Stefan Beller
@ 2017-09-22 22:58     ` Jonathan Nieder
  2017-09-23 23:52       ` Junio C Hamano
  0 siblings, 1 reply; 12+ messages in thread
From: Jonathan Nieder @ 2017-09-22 22:58 UTC (permalink / raw)
  To: Stefan Beller; +Cc: git

Stefan Beller wrote:

> Reported-by: Lars Schneider <larsxschneider@gmail.com>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>  Documentation/config.txt | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
>
> Jonathan writes:

>> You'll want to update Documentation/gitmodules.txt, too.
>
> No. /grumpycat
>
> It should already be fine, as I read it as if it is only relevant to 
> "git submodule update" there, already:
>
>   submodule.<name>.update::
> 	Defines the default update procedure for the named submodule,
> 	i.e. how the submodule is updated by "git submodule update"
> 	command in the superproject. This is only used by `git
> 	submodule init` to initialize the configuration variable of
> 	the same name. Allowed values here are 'checkout', 'rebase',
> 	'merge' or 'none'. See description of 'update' command in
> 	linkgit:git-submodule[1] for their meaning. Note that the
> 	'!command' form is intentionally ignored here for security
> 	reasons.

I disagree.  Actually, I think the git-config(1) blurb could just
point here, and that the text here ought to be clear about what
commands it affects and how an end user would use it.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCHv2] Documentation/config: clarify the meaning of submodule.<name>.update
  2017-09-22 22:58     ` Jonathan Nieder
@ 2017-09-23 23:52       ` Junio C Hamano
  2017-09-25 19:10         ` [PATCH] Documentation: consolidate submodule.<name>.update Stefan Beller
  0 siblings, 1 reply; 12+ messages in thread
From: Junio C Hamano @ 2017-09-23 23:52 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Stefan Beller, git

Jonathan Nieder <jrnieder@gmail.com> writes:

> Stefan Beller wrote:
>
>> Reported-by: Lars Schneider <larsxschneider@gmail.com>
>> Signed-off-by: Stefan Beller <sbeller@google.com>
>> ---
>>  Documentation/config.txt | 12 ++++++++----
>>  1 file changed, 8 insertions(+), 4 deletions(-)
>>
>> Jonathan writes:
>
>>> You'll want to update Documentation/gitmodules.txt, too.
>>
>> No. /grumpycat
>>
>> It should already be fine, as I read it as if it is only relevant to 
>> "git submodule update" there, already:
>>
>>   submodule.<name>.update::
>> 	Defines the default update procedure for the named submodule,
>> 	i.e. how the submodule is updated by "git submodule update"
>> 	command in the superproject. This is only used by `git
>> 	submodule init` to initialize the configuration variable of
>> 	the same name. Allowed values here are 'checkout', 'rebase',
>> 	'merge' or 'none'. See description of 'update' command in
>> 	linkgit:git-submodule[1] for their meaning. Note that the
>> 	'!command' form is intentionally ignored here for security
>> 	reasons.
>
> I disagree.  Actually, I think the git-config(1) blurb could just
> point here, and that the text here ought to be clear about what
> commands it affects and how an end user would use it.

I tend to agree with the consolidation.  It's not like this patch
makes things worse, but as we've already diverted our attention to
this area and took time to review, if it is a simple update we can
make before we apply, that is better than having to remember and
come back to patch the result of this step further.

In any case, thanks for reporting, attempting to improve and
reviewing, all.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] Documentation: consolidate submodule.<name>.update
  2017-09-23 23:52       ` Junio C Hamano
@ 2017-09-25 19:10         ` Stefan Beller
  2017-09-25 19:17           ` Brandon Williams
  0 siblings, 1 reply; 12+ messages in thread
From: Stefan Beller @ 2017-09-25 19:10 UTC (permalink / raw)
  To: gitster; +Cc: git, jrnieder, sbeller

Have one place to explain the effects of setting submodule.<name>.update
instead of two.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
>> I disagree.  Actually, I think the git-config(1) blurb could just
>> point here, and that the text here ought to be clear about what
>> commands it affects and how an end user would use it.
>
> I tend to agree with the consolidation.

Something like this?

 Documentation/config.txt     |  9 +--------
 Documentation/gitmodules.txt | 20 +++++++++++---------
 2 files changed, 12 insertions(+), 17 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 1ac0ae6adb..0d5a296b6c 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -3085,14 +3085,7 @@ submodule.<name>.url::
 	See linkgit:git-submodule[1] and linkgit:gitmodules[5] for details.
 
 submodule.<name>.update::
-	The method by which a submodule is updated by 'git submodule update',
-	which is the only affected command, others such as
-	'git checkout --recurse-submodules' are unaffected. It exists for
-	historical reasons, when 'git submodule' was the only command to
-	interact with submodules; settings like `submodule.active`
-	and `pull.rebase` are more specific. It is populated by
-	`git submodule init` from the linkgit:gitmodules[5] file.
-	See description of 'update' command in linkgit:git-submodule[1].
+	See `submodule.<name>.update` in linkgit:gitmodules[5].
 
 submodule.<name>.branch::
 	The remote branch name for a submodule, used by `git submodule
diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt
index db5d47eb19..d156dee387 100644
--- a/Documentation/gitmodules.txt
+++ b/Documentation/gitmodules.txt
@@ -38,15 +38,17 @@ submodule.<name>.url::
 In addition, there are a number of optional keys:
 
 submodule.<name>.update::
-	Defines the default update procedure for the named submodule,
-	i.e. how the submodule is updated by "git submodule update"
-	command in the superproject. This is only used by `git
-	submodule init` to initialize the configuration variable of
-	the same name. Allowed values here are 'checkout', 'rebase',
-	'merge' or 'none'. See description of 'update' command in
-	linkgit:git-submodule[1] for their meaning. Note that the
-	'!command' form is intentionally ignored here for security
-	reasons.
+	The method by which a submodule is updated by 'git submodule update',
+	which is the only affected command, others such as
+	'git checkout --recurse-submodules' are unaffected. It exists for
+	historical reasons, when 'git submodule' was the only command to
+	interact with submodules; settings like `submodule.active`
+	and `pull.rebase` are more specific. It is copied to the config
+	by `git submodule init` from the .gitmodules file.
+	Allowed values here are 'checkout', 'rebase', 'merge' or 'none'.
+	See description of 'update' command in linkgit:git-submodule[1]
+	for their meaning. Note that the '!command' form is intentionally
+	ignored here for security reasons.
 
 submodule.<name>.branch::
 	A remote branch name for tracking updates in the upstream submodule.
-- 
2.14.0.rc0.3.g6c2e499285


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] Documentation: consolidate submodule.<name>.update
  2017-09-25 19:10         ` [PATCH] Documentation: consolidate submodule.<name>.update Stefan Beller
@ 2017-09-25 19:17           ` Brandon Williams
  2017-09-25 22:18             ` Stefan Beller
  2017-09-26  1:30             ` Junio C Hamano
  0 siblings, 2 replies; 12+ messages in thread
From: Brandon Williams @ 2017-09-25 19:17 UTC (permalink / raw)
  To: Stefan Beller; +Cc: gitster, git, jrnieder

On 09/25, Stefan Beller wrote:
> Have one place to explain the effects of setting submodule.<name>.update
> instead of two.
> 
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
> >> I disagree.  Actually, I think the git-config(1) blurb could just
> >> point here, and that the text here ought to be clear about what
> >> commands it affects and how an end user would use it.
> >
> > I tend to agree with the consolidation.
> 
> Something like this?

I like the consolidation, its easier to keep up to date when its only in
one place.

> 
>  Documentation/config.txt     |  9 +--------
>  Documentation/gitmodules.txt | 20 +++++++++++---------
>  2 files changed, 12 insertions(+), 17 deletions(-)
> 
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 1ac0ae6adb..0d5a296b6c 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -3085,14 +3085,7 @@ submodule.<name>.url::
>  	See linkgit:git-submodule[1] and linkgit:gitmodules[5] for details.
>  
>  submodule.<name>.update::
> -	The method by which a submodule is updated by 'git submodule update',
> -	which is the only affected command, others such as
> -	'git checkout --recurse-submodules' are unaffected. It exists for
> -	historical reasons, when 'git submodule' was the only command to
> -	interact with submodules; settings like `submodule.active`
> -	and `pull.rebase` are more specific. It is populated by
> -	`git submodule init` from the linkgit:gitmodules[5] file.
> -	See description of 'update' command in linkgit:git-submodule[1].
> +	See `submodule.<name>.update` in linkgit:gitmodules[5].
>  
>  submodule.<name>.branch::
>  	The remote branch name for a submodule, used by `git submodule
> diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt
> index db5d47eb19..d156dee387 100644
> --- a/Documentation/gitmodules.txt
> +++ b/Documentation/gitmodules.txt
> @@ -38,15 +38,17 @@ submodule.<name>.url::
>  In addition, there are a number of optional keys:
>  
>  submodule.<name>.update::
> -	Defines the default update procedure for the named submodule,
> -	i.e. how the submodule is updated by "git submodule update"
> -	command in the superproject. This is only used by `git
> -	submodule init` to initialize the configuration variable of
> -	the same name. Allowed values here are 'checkout', 'rebase',
> -	'merge' or 'none'. See description of 'update' command in
> -	linkgit:git-submodule[1] for their meaning. Note that the
> -	'!command' form is intentionally ignored here for security
> -	reasons.
> +	The method by which a submodule is updated by 'git submodule update',
> +	which is the only affected command, others such as
> +	'git checkout --recurse-submodules' are unaffected. It exists for
> +	historical reasons, when 'git submodule' was the only command to
> +	interact with submodules; settings like `submodule.active`
> +	and `pull.rebase` are more specific. It is copied to the config
> +	by `git submodule init` from the .gitmodules file.
> +	Allowed values here are 'checkout', 'rebase', 'merge' or 'none'.
> +	See description of 'update' command in linkgit:git-submodule[1]
> +	for their meaning. Note that the '!command' form is intentionally
> +	ignored here for security reasons.

This probably needs to be tweaked a bit to say that the '!command' form
is ignored by submodule init, in that it isn't copied over from the
.gitmodules file, but if it is configured in your config it will be
respected by 'submodule update'.

>  
>  submodule.<name>.branch::
>  	A remote branch name for tracking updates in the upstream submodule.
> -- 
> 2.14.0.rc0.3.g6c2e499285
> 

-- 
Brandon Williams

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Documentation: consolidate submodule.<name>.update
  2017-09-25 19:17           ` Brandon Williams
@ 2017-09-25 22:18             ` Stefan Beller
  2017-09-25 22:22               ` Jonathan Nieder
  2017-09-26  1:30             ` Junio C Hamano
  1 sibling, 1 reply; 12+ messages in thread
From: Stefan Beller @ 2017-09-25 22:18 UTC (permalink / raw)
  To: Brandon Williams; +Cc: Junio C Hamano, git@vger.kernel.org, Jonathan Nieder

On Mon, Sep 25, 2017 at 12:17 PM, Brandon Williams <bmwill@google.com> wrote:
> On 09/25, Stefan Beller wrote:
>> Have one place to explain the effects of setting submodule.<name>.update
>> instead of two.
>>
>> Signed-off-by: Stefan Beller <sbeller@google.com>
>> ---
>> >> I disagree.  Actually, I think the git-config(1) blurb could just
>> >> point here, and that the text here ought to be clear about what
>> >> commands it affects and how an end user would use it.
>> >
>> > I tend to agree with the consolidation.
>>
>> Something like this?
>
> I like the consolidation, its easier to keep up to date when its only in
> one place.

After thinking about it further, I'd like to withdraw this patch
for now.

The reason is that this would also forward point to
git-submodule.txt, such that we'd still have 2 places
in which it is explained. The current situation with explaining
it in 3 places is not too bad as each place hints at their specific
circumstance:
git-submodule.txt explains the values to set itself.
gitmodules.txt explains what the possible fields in that file are,
  and regarding this field it points out it is ignored in the init call.
config.txt: actually describe the effects of the setting.

I think we can keep it as-is for now.

Thanks,
Stefan

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Documentation: consolidate submodule.<name>.update
  2017-09-25 22:18             ` Stefan Beller
@ 2017-09-25 22:22               ` Jonathan Nieder
  2017-09-25 22:42                 ` Stefan Beller
  0 siblings, 1 reply; 12+ messages in thread
From: Jonathan Nieder @ 2017-09-25 22:22 UTC (permalink / raw)
  To: Stefan Beller; +Cc: Brandon Williams, Junio C Hamano, git@vger.kernel.org

Stefan Beller wrote:
> On Mon, Sep 25, 2017 at 12:17 PM, Brandon Williams <bmwill@google.com> wrote:
>> On 09/25, Stefan Beller wrote:

>>> Have one place to explain the effects of setting submodule.<name>.update
>>> instead of two.
>>>
>>> Signed-off-by: Stefan Beller <sbeller@google.com>
>>> ---
>>>>> I disagree.  Actually, I think the git-config(1) blurb could just
>>>>> point here, and that the text here ought to be clear about what
>>>>> commands it affects and how an end user would use it.
>>>>
>>>> I tend to agree with the consolidation.
>>>
>>> Something like this?
>>
>> I like the consolidation, its easier to keep up to date when its only in
>> one place.
>
> After thinking about it further, I'd like to withdraw this patch
> for now.
>
> The reason is that this would also forward point to
> git-submodule.txt, such that we'd still have 2 places
> in which it is explained. The current situation with explaining
> it in 3 places is not too bad as each place hints at their specific
> circumstance:
> git-submodule.txt explains the values to set itself.
> gitmodules.txt explains what the possible fields in that file are,
>   and regarding this field it points out it is ignored in the init call.
> config.txt: actually describe the effects of the setting.
>
> I think we can keep it as-is for now.

Sorry, I got lost.  Which state is as-is?

As a user, how do I find out what submodule.*.update is going to do
and which commands will respect it?

Maybe we should work on first wordsmithing the doc and then figuring
out where it goes?  The current state of the doc with (?) three
different texts, all wrong, doesn't seem like a good state to
preserve.

Thanks,
Jonathan

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Documentation: consolidate submodule.<name>.update
  2017-09-25 22:22               ` Jonathan Nieder
@ 2017-09-25 22:42                 ` Stefan Beller
  2017-09-25 23:44                   ` Jonathan Nieder
  0 siblings, 1 reply; 12+ messages in thread
From: Stefan Beller @ 2017-09-25 22:42 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Brandon Williams, Junio C Hamano, git@vger.kernel.org

On Mon, Sep 25, 2017 at 3:22 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Stefan Beller wrote:
>> On Mon, Sep 25, 2017 at 12:17 PM, Brandon Williams <bmwill@google.com> wrote:
>>> On 09/25, Stefan Beller wrote:
>
>>>> Have one place to explain the effects of setting submodule.<name>.update
>>>> instead of two.
>>>>
>>>> Signed-off-by: Stefan Beller <sbeller@google.com>
>>>> ---
>>>>>> I disagree.  Actually, I think the git-config(1) blurb could just
>>>>>> point here, and that the text here ought to be clear about what
>>>>>> commands it affects and how an end user would use it.
>>>>>
>>>>> I tend to agree with the consolidation.
>>>>
>>>> Something like this?
>>>
>>> I like the consolidation, its easier to keep up to date when its only in
>>> one place.
>>
>> After thinking about it further, I'd like to withdraw this patch
>> for now.
>>
>> The reason is that this would also forward point to
>> git-submodule.txt, such that we'd still have 2 places
>> in which it is explained. The current situation with explaining
>> it in 3 places is not too bad as each place hints at their specific
>> circumstance:
>> git-submodule.txt explains the values to set itself.
>> gitmodules.txt explains what the possible fields in that file are,
>>   and regarding this field it points out it is ignored in the init call.
>> config.txt: actually describe the effects of the setting.
>>
>> I think we can keep it as-is for now.
>
> Sorry, I got lost.  Which state is as-is?

By as-is I refer to origin/pu.

> As a user, how do I find out what submodule.*.update is going to do
> and which commands will respect it?

The user would discover it via 'man git-config' I would assume, which
covers any config variable? It also directs to git-submodule which is
more detailed, but the text there is suitable for the casual reader.
(pu has origin/sb/doc-config-submodule-update)

> Maybe we should work on first wordsmithing the doc and then figuring
> out where it goes?  The current state of the doc with (?) three
> different texts,

such that each different text highlights each locations purpose.

> all wrong,

Care to spell out this bold claim?

Thanks,
Stefan

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Documentation: consolidate submodule.<name>.update
  2017-09-25 22:42                 ` Stefan Beller
@ 2017-09-25 23:44                   ` Jonathan Nieder
  0 siblings, 0 replies; 12+ messages in thread
From: Jonathan Nieder @ 2017-09-25 23:44 UTC (permalink / raw)
  To: Stefan Beller; +Cc: Brandon Williams, Junio C Hamano, git@vger.kernel.org

Stefan Beller wrote:

> By as-is I refer to origin/pu.

Ah, okay.  I'm not in that habit because pu frequently loses topics
--- it's mostly useful as a tool to (1) distribute topic branches and
(2) check whether they are compatible with each other.

[...]
> On Mon, Sep 25, 2017 at 3:22 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:

>> Maybe we should work on first wordsmithing the doc and then figuring
>> out where it goes?  The current state of the doc with (?) three
>> different texts,
>
> such that each different text highlights each locations purpose.
>
>> all wrong,
>
> Care to spell out this bold claim?

In the state of "master", "man git-config" tells me that

	[submodule "<name>"]
		update = ...

sets the "default update procedure for a submodule".  It suggests that
I read about "git submodule update" in git-submodule(1) for more
details.  This is problematic because

 1. It doesn't tell me when I should and should not set it.
 2. I would expect "git checkout --recurse-submodules" to respect the
    default update procedure.
 3. It doesn't tell me what commands like "git fetch
    --recurse-submodules" will do.
 4. It doesn't point me to better alternatives, when this
    configuration doesn't fit my use case.

"man git-submodule" doesn't have a section on submodule.<name>.update.
It has references scattered throughout the manpage.  It only tells me
how the setting affects the "git submodule update" command and doesn't
address the problems (1)-(4).

"man gitmodules" also refers to this setting as the "default update
procedure for the named submodule".  It doesn't answer the questions
(1)-(4) either.

Thanks and hope that helps,
Jonathan

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Documentation: consolidate submodule.<name>.update
  2017-09-25 19:17           ` Brandon Williams
  2017-09-25 22:18             ` Stefan Beller
@ 2017-09-26  1:30             ` Junio C Hamano
  1 sibling, 0 replies; 12+ messages in thread
From: Junio C Hamano @ 2017-09-26  1:30 UTC (permalink / raw)
  To: Brandon Williams; +Cc: Stefan Beller, git, jrnieder

Brandon Williams <bmwill@google.com> writes:

>> +	The method by which a submodule is updated by 'git submodule update',
>> +	which is the only affected command, others such as
>> +	'git checkout --recurse-submodules' are unaffected. It exists for
>> +	historical reasons, when 'git submodule' was the only command to
>> +	interact with submodules; settings like `submodule.active`
>> +	and `pull.rebase` are more specific. It is copied to the config
>> +	by `git submodule init` from the .gitmodules file.
>> +	Allowed values here are 'checkout', 'rebase', 'merge' or 'none'.
>> +	See description of 'update' command in linkgit:git-submodule[1]
>> +	for their meaning. Note that the '!command' form is intentionally
>> +	ignored here for security reasons.
>
> This probably needs to be tweaked a bit to say that the '!command' form
> is ignored by submodule init, in that it isn't copied over from the
> .gitmodules file, but if it is configured in your config it will be
> respected by 'submodule update'.

I do not think gitmodules.txt is the place to say anything more than
what Stefan's patch says above.  Perhaps config.txt should mention
that in addition to what the variable with the same in .gitmodules
can take, it is allowed to use the !command form.

IOW, in config.txt

	submodule.<name>.update::
		Specifies how 'git submodule update' should work on
		the named submodule.  In addition to the values that
		can be specified in (and copied from) `.gitmodules`
		(see linkgit:gitmodules[5]), `!command` form can
		also be used.

or something, perhaps?

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2017-09-26  1:31 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-22 21:28 [PATCH] Documentation/config: clarify the meaning of submodule.<name>.update Stefan Beller
2017-09-22 21:37 ` Jonathan Nieder
2017-09-22 22:52   ` [PATCHv2] " Stefan Beller
2017-09-22 22:58     ` Jonathan Nieder
2017-09-23 23:52       ` Junio C Hamano
2017-09-25 19:10         ` [PATCH] Documentation: consolidate submodule.<name>.update Stefan Beller
2017-09-25 19:17           ` Brandon Williams
2017-09-25 22:18             ` Stefan Beller
2017-09-25 22:22               ` Jonathan Nieder
2017-09-25 22:42                 ` Stefan Beller
2017-09-25 23:44                   ` Jonathan Nieder
2017-09-26  1:30             ` Junio C Hamano

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).