Re: [PATCH 6/8] Doc/gitsubmodules: improve readability of certain lines

[Kaartic Sivaraam]

On Wednesday 10 January 2018 01:01 AM, Stefan Beller wrote:
  The submodule's `$GIT_DIR/config` file would come into play when running
  `git push --recurse-submodules=check` in the superproject, as this would
 @@ -107,13 +108,13 @@ If the submodule is not yet initialized, then the 
 configuration
  inside the submodule does not exist yet, so configuration where to
  obtain the submodule from is configured here for example.

>>
>> I caught this in the context while replying. "so configuration where to
>> obtain the submodule from is configured here for example." doesn't seem
>> to read well. Maybe removing configuration from the sentence will make
>> it sound better?
>>

I'm going to make this change.


>>
 - * the `.gitmodules` file inside the superproject. Additionally to the
 -   required mapping between submodule's name and path, a project usually
 + * The `.gitmodules` file inside the superproject. Additionally, if 
 mapping
 +   is required between a submodule's name and its path, a project usually
>>>
>>> This changes meaning, originally it tries to say:
>>>
>>> * it requires mapping path <-> names.
>>
>> I get this ...
>>
>>> * but there can be more.
>>
>> ... but not this. Did the previous version really try to say this?
>> Anyways how does this sound?
> 
> Well that was me being very sloppy trying to say that there might be
> submodule..{url, ignored, shallow} settings which just happen to
> be there.
> 
>>   * The `.gitmodules` file inside the superproject. A project usually
>> uses this file to suggest defaults for the upstream collection
>> of repositories for the mapping that is required between a
>> submodule's name and its path.
>>
>> I think it conveys the "it requires mapping path <-> names." correctly
>> but doesn't convey the "but there can be more." part. I'm not sure how
>> to get that into the sentence, correctly.
> 
> I did not consider that part the important part, hence my sloppiness.
> Sorry for the confusion.
> 
> My main point was to say that the mapping is the important part and
> must be found in the .gitmodules file, otherwise we do not consider
> it a submodule (for whatever "it" is, maybe a gitlink at a path=name).
> 

So, I'm going to use the version that I specified above as I think it
seems to convey that clearly (at least to me),

The `.gitmodules` file inside the superproject. A project usually
uses this file to suggest defaults for the upstream collection
of repositories for the mapping that is required between a
submodule's name and its path.

Let me know, if there are issues.

Thanks,
Kaartic



signature.asc
Description: OpenPGP digital signature

Re: [PATCH 6/8] Doc/gitsubmodules: improve readability of certain lines

[Stefan Beller]

>>>  The submodule's `$GIT_DIR/config` file would come into play when running
>>>  `git push --recurse-submodules=check` in the superproject, as this would
>>> @@ -107,13 +108,13 @@ If the submodule is not yet initialized, then the 
>>> configuration
>>>  inside the submodule does not exist yet, so configuration where to
>>>  obtain the submodule from is configured here for example.
>>>
>
> I caught this in the context while replying. "so configuration where to
> obtain the submodule from is configured here for example." doesn't seem
> to read well. Maybe removing configuration from the sentence will make
> it sound better?
>
>
>>> - * the `.gitmodules` file inside the superproject. Additionally to the
>>> -   required mapping between submodule's name and path, a project usually
>>> + * The `.gitmodules` file inside the superproject. Additionally, if mapping
>>> +   is required between a submodule's name and its path, a project usually
>>
>> This changes meaning, originally it tries to say:
>>
>> * it requires mapping path <-> names.
>
> I get this ...
>
>> * but there can be more.
>
> ... but not this. Did the previous version really try to say this?
> Anyways how does this sound?

Well that was me being very sloppy trying to say that there might be
submodule..{url, ignored, shallow} settings which just happen to
be there.

>   * The `.gitmodules` file inside the superproject. A project usually
> uses this file to suggest defaults for the upstream collection
> of repositories for the mapping that is required between a
> submodule's name and its path.
>
> I think it conveys the "it requires mapping path <-> names." correctly
> but doesn't convey the "but there can be more." part. I'm not sure how
> to get that into the sentence, correctly.

I did not consider that part the important part, hence my sloppiness.
Sorry for the confusion.

My main point was to say that the mapping is the important part and
must be found in the .gitmodules file, otherwise we do not consider
it a submodule (for whatever "it" is, maybe a gitlink at a path=name).

Stefan

Re: [PATCH 6/8] Doc/gitsubmodules: improve readability of certain lines

[Kaartic Sivaraam]

On Tuesday 09 January 2018 12:19 AM, Stefan Beller wrote:
> On Sat, Jan 6, 2018 at 10:46 AM, Kaartic Sivaraam
>  wrote:
>>
>> - * The command line for those commands that support taking submodule
>> -   specifications. Most commands have a boolean flag '--recurse-submodules
>> -   whether to recurse into submodules. Examples are `ls-files` or 
>> `checkout`.
>> + * The command line arguments of those commands that support taking 
>> submodule
>> +   specifications. Most commands have a boolean flag '--recurse-submodules'
>> +   which specify whether they should recurse into submodules. Examples are
>> +   `ls-files` or `checkout`.
>> Some commands take enums, such as `fetch` and `push`, where you can
>> specify how submodules are affected.
>>
>> @@ -90,8 +91,8 @@ Submodule operations can be configured using the following 
>> mechanisms
>>  For example an effect from the submodule's `.gitignore` file
>>  would be observed when you run `git status --ignore-submodules=none` in
>>  the superproject. This collects information from the submodule's working
>> -directory by running `status` in the submodule, which does pay attention
>> -to its `.gitignore` file.
>> +directory by running `status` in the submodule while paying attention
>> +to the `.gitignore` file of the submodule.
> 
> Both are grammatically correct and expressive, thanks!
>

You're welcome!


>>  +
> 
> Extra spurious line?
>

No. That's a "real" plus in the document that's usually present between
paragraphs :) I think I now get why Junio suggests people to review
patches in context (possibly, by applying them) ;)


>>  The submodule's `$GIT_DIR/config` file would come into play when running
>>  `git push --recurse-submodules=check` in the superproject, as this would
>> @@ -107,13 +108,13 @@ If the submodule is not yet initialized, then the 
>> configuration
>>  inside the submodule does not exist yet, so configuration where to
>>  obtain the submodule from is configured here for example.
>>

I caught this in the context while replying. "so configuration where to
obtain the submodule from is configured here for example." doesn't seem
to read well. Maybe removing configuration from the sentence will make
it sound better?


>> - * the `.gitmodules` file inside the superproject. Additionally to the
>> -   required mapping between submodule's name and path, a project usually
>> + * The `.gitmodules` file inside the superproject. Additionally, if mapping
>> +   is required between a submodule's name and its path, a project usually
> 
> This changes meaning, originally it tries to say:
> 
> * it requires mapping path <-> names.

I get this ...

> * but there can be more.
> 

... but not this. Did the previous version really try to say this?
Anyways how does this sound?

  * The `.gitmodules` file inside the superproject. A project usually
uses this file to suggest defaults for the upstream collection
of repositories for the mapping that is required between a
submodule's name and its path.

I think it conveys the "it requires mapping path <-> names." correctly
but doesn't convey the "but there can be more." part. I'm not sure how
to get that into the sentence, correctly.



signature.asc
Description: OpenPGP digital signature

Re: [PATCH 6/8] Doc/gitsubmodules: improve readability of certain lines

[Stefan Beller]

On Sat, Jan 6, 2018 at 10:46 AM, Kaartic Sivaraam
 wrote:
> Signed-off-by: Kaartic Sivaraam 
> ---
>  Documentation/gitsubmodules.txt | 19 ++-
>  1 file changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/Documentation/gitsubmodules.txt b/Documentation/gitsubmodules.txt
> index 745a3838e..339fb73db 100644
> --- a/Documentation/gitsubmodules.txt
> +++ b/Documentation/gitsubmodules.txt
> @@ -76,9 +76,10 @@ The configuration of submodules
>  Submodule operations can be configured using the following mechanisms
>  (from highest to lowest precedence):
>
> - * The command line for those commands that support taking submodule
> -   specifications. Most commands have a boolean flag '--recurse-submodules
> -   whether to recurse into submodules. Examples are `ls-files` or `checkout`.
> + * The command line arguments of those commands that support taking submodule
> +   specifications. Most commands have a boolean flag '--recurse-submodules'
> +   which specify whether they should recurse into submodules. Examples are
> +   `ls-files` or `checkout`.
> Some commands take enums, such as `fetch` and `push`, where you can
> specify how submodules are affected.
>
> @@ -90,8 +91,8 @@ Submodule operations can be configured using the following 
> mechanisms
>  For example an effect from the submodule's `.gitignore` file
>  would be observed when you run `git status --ignore-submodules=none` in
>  the superproject. This collects information from the submodule's working
> -directory by running `status` in the submodule, which does pay attention
> -to its `.gitignore` file.
> +directory by running `status` in the submodule while paying attention
> +to the `.gitignore` file of the submodule.

Both are grammatically correct and expressive, thanks!

>  +

Extra spurious line?

>  The submodule's `$GIT_DIR/config` file would come into play when running
>  `git push --recurse-submodules=check` in the superproject, as this would
> @@ -107,13 +108,13 @@ If the submodule is not yet initialized, then the 
> configuration
>  inside the submodule does not exist yet, so configuration where to
>  obtain the submodule from is configured here for example.
>
> - * the `.gitmodules` file inside the superproject. Additionally to the
> -   required mapping between submodule's name and path, a project usually
> + * The `.gitmodules` file inside the superproject. Additionally, if mapping
> +   is required between a submodule's name and its path, a project usually

This changes meaning, originally it tries to say:

* it requires mapping path <-> names.
* but there can be more.

whereas the new lines are:

* mapping is optional
* there can be more.

> uses this file to suggest defaults for the upstream collection
> of repositories.
>  +
> -This file mainly serves as the mapping between name and path in
> -the superproject, such that the submodule's Git directory can be
> +This file mainly serves as the mapping between the name and path of 
> submodules
> +in the superproject, such that the submodule's Git directory can be
>  located.

makes sense!

Thanks,
Stefan

>  +
>  If the submodule has never been initialized, this is the only place
> --
> 2.16.0.rc0.223.g4a4ac8367
>

Re: [PATCH 6/8] Doc/gitsubmodules: improve readability of certain lines

[Eric Sunshine]

On Sat, Jan 6, 2018 at 1:46 PM, Kaartic Sivaraam
 wrote:
> Signed-off-by: Kaartic Sivaraam 
> ---
> diff --git a/Documentation/gitsubmodules.txt b/Documentation/gitsubmodules.txt
> @@ -76,9 +76,10 @@ The configuration of submodules
> - * The command line for those commands that support taking submodule
> -   specifications. Most commands have a boolean flag '--recurse-submodules
> -   whether to recurse into submodules. Examples are `ls-files` or `checkout`.
> + * The command line arguments of those commands that support taking submodule
> +   specifications. Most commands have a boolean flag '--recurse-submodules'
> +   which specify whether they should recurse into submodules. Examples are
> +   `ls-files` or `checkout`.

So, this is addressing issues pointed out in my review of 4/8. It's
actually fixing a problem -- missing closing quote -- introduced by
4/8. Therefore, it probably would make sense either to move this hunk
into 4/8 or drop 4/8 altogether.