Re: [PATCH 0/8] Doc/submodules: a few updates

[Stefan Beller <sbeller@google.com>]

On Tue, Jan 9, 2018 at 9:06 AM, Kaartic Sivaraam
<kaartic.sivaraam@gmail.com> wrote:
> On Tuesday 09 January 2018 12:38 AM, Stefan Beller wrote:
>> On Sat, Jan 6, 2018 at 10:46 AM, Kaartic Sivaraam
>> <kaartic.sivaraam@gmail.com> wrote:
>>
>> While small patches are really appreciated for code (bisect, automated
>> testing, and
>> the general difficulty to reason about code, as a very small change
>> may affect the whole
>> code base), I am not sure if they benefit in documentation.
>> Documentation is a rather
>> local human readable thing, so by changing one sentence we don't
>> affect the understanding
>> of documentation at a completely unrelated place.
>>
>> Also it helps to read more than just sentence fragments, i.e. I tried
>> looking at the
>> whole paragraph for review. May I suggest to squash them all and
>> resend as one patch?
>>
>
> I wouldn't mind that. I thought it might be easy to find to find the
> parts I changed when the patches are small. So, I sent them without
> squashing them together. In case you feel it's not worth, let me know so
> I'll squash them in.
>
> BTW, in case I did squash them in, would it be nice to keep the commit
> subjects of the current patch series as bullet points in the unified
> commit message?

Sure.

>> I wonder if this indicates a lack of documentation when the active
>> flags were introduced.
>> They are found in 'man git config', but maybe we need to spell them
>> out explicitly
>> in the submodule related docs.
>>
>
> Possibly. So, why not in Documentation/gitsubmodules! Here's a replaced
> version of that paragraph,
>
>     * The configuration file `$GIT_DIR/config` in the superproject.
>    Typically this file is used to specify whether the submodule
>    is recursed into at all via the `active` flag for example. A
>    submodule is considered active if `submodule.<name>.url` is set
>    or if the submodules path is present in `submodule.active` or
>    if `submodule.<name>.url` is set.

I wonder if we'd want to give an example later, and first describe the
mechanics precisely:

   The configuration file `$GIT_DIR/config` in the superproject.
    Git only recurses into active submodules. A submodule is
    considered active (a) if `submodule.<name>.active` is set
    or (b) if the submodules path is matches the pathsepc in
    `submodule.active` or (c) if `submodule.<name>.url` is set.
    (c) is a historical artefact and will be ignored if the pathspec
    set in (b) excludes the submodule. For example:

    [submodule "foo"]
        active = false
        url = https://example.org/foo
    [submodule "bar"]
        active = true
        url = https://example.org/bar
    [submodule "baz"]
        url = https://example.org/baz

    In the above config only the submodule bar and baz are active,
    bar due to (a) and baz due to (c). Another example

    [submodule "foo"]
        active = true
        url = https://example.org/foo
    [submodule "bar"]
        url = https://example.org/bar
    [submodule "baz"]
        url = https://example.org/baz
    [submodule "bob"]
        ignore = true
    [submodule]
        active = b*
        active = (:exclude) baz

    In here all submodules except baz (foo, bar, bob) are active.
    'foo' due to its own active flag and all the others due to the
    submodule active pathspec, which specifies that any submodule
    starting with 'b' except 'baz' are also active, no matter if the .url field
    is present.

>>> 2.
>>>
>>>  man git submodule
>>>
>>>        update
>>>            ...
>>>
>>>            checkout
>>>                ....
>>>
>>>                If --force is specified, the submodule will be checked out (using git checkout --force if appropriate), even if the commit
>>>                specified in the index of the containing repository already matches the commit checked out in the submodule.
>>>
>>> I'm not sure this is conveying all the information it should be conveying.
>>> It seems to making the user wonder, "How at all does 'git submodule update --force'
>>> differs from 'git submodule update'?" also "using git checkout --force if appropriate"
>>> seems to be invoking all sorts confusion as "appropriate" is superfluous.
>>
>> When "submodule update" is invoked with the `--force` flag, that flag is passed
>> on to the 'checkout' operation. If you do not give the --force, then
>> the checkout
>> will also be done without --force.
>>
>
> If that's the case then shouldn't the "if appropriate" part of "(using
> git checkout --force if appropriate)" be dropped? That seems to make it
> clear, at least for me. Or is intended as '--force' will not be passed
> to git checkout all the time?
>

Yes, essentially we only pass the force flag when we were given the force flag
("when appropriate" :) Not sure how to say that otherwise. But dropping sounds
good)

Stefan