Thread (8 messages) flat view 8 messages, 5 authors, 2021-01-06

Re: [PATCH] gitmodules.txt: fix 'GIT_WORK_TREE' variable name

From: Martin Ågren <hidden>
Date: 2021-01-02 21:35:28

On Sat, 2 Jan 2021 at 20:39, Philippe Blain via GitGitGadget
[off-list ref] wrote:
From: Philippe Blain <redacted>

'gitmodules.txt' is a guide about the '.gitmodules' file that describes
submodules properties, and that file must exist at the root of the
repository. This was clarified in e5b5c1d2cf (Document clarification:
gitmodules, gitattributes, 2008-08-31).

However, that commit mistakenly uses the non-existing environment
variable 'GIT_WORK_DIR' to refer to the root of the repository.
Good catch! I wonder what we should conclude from this having gone
unreported for so long.
Fix that by using the correct variable, 'GIT_WORK_TREE'. Take the
opportunity to modernize and improve the formatting of that guide.
It's a small correctness fix and some prettifying while at it. While I
read the diff and realized that it was more than just one or two
asciidoc tweaks in the immediate vicinity, I started wondering if this
should be presented the other way round: "Let's update the formatting
and fix s/DIR/TREE/ while at it." Or to split it up. But I don't think
it's worth thinking too much about -- what you have looks good to me.
quoted hunk ↗ jump to hunk
@@ -32,14 +32,14 @@ submodule.<name>.path::
Just above this point, you have another s/\.gitmodules/`&`/ waiting to
be fixed.
 submodule.<name>.url::
        Defines a URL from which the submodule repository can be cloned.
        This may be either an absolute URL ready to be passed to
        dirty;; All changes to the submodule's work tree will be ignored, only
-           committed differences between the HEAD of the submodule and its
+           committed differences between the 'HEAD' of the submodule and its
            recorded state in the superproject are taken into account.
`git grep -B10 HEAD CodingGuidelines` suggests this should be `HEAD`,
not 'HEAD'. Maybe you followed style -- there's an instance of 'HEAD'
earlier. I think both should be `HEAD`.
-If this option is also present in the submodules entry in .git/config
+If this option is also present in the submodules entry in `.git/config`
 of the superproject, the setting there will override the one found in
-.gitmodules.
+`.gitmodules`.
Should "submodules entry" be "submodule's entry"? I've never worked with
submodules, but that reading somehow seems more natural. (There are two
hits for "submodules entry" in this document -- both might be worth
looking at.)

This patch looks good to me. It might be worthwhile to reroll
to address the naked .gitmodules around line 30 and the two 'HEAD' while
you're in the area. As for the last comment above, it could well be
that the proper response is "no, you're wrong".

Martin
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help