Re: [PATCH] gitmodules.txt: fix 'GIT_WORK_TREE' variable name
- From
- Martin Ågren <martin.agren@gmail.com>
- Date
- Jan 2, 2021, 21:34 UTC
- Message-ID
- <CAN0heSqC3K6pJOr2ztz56+ZpKaMomA28rc4W5x8n0cC3K-rVgQ@mail.gmail.com>
- In-Reply-To
- <pull.942.git.git.1609616245412.gitgitgadget@gmail.com>
On Sat, 2 Jan 2021 at 20:39, Philippe Blain via GitGitGadget <gitgitgadget@gmail.com> wrote:
Show 10 quoted lines
> > From: Philippe Blain <levraiphilippeblain@gmail.com> > > '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.
> @@ -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`.
Show 5 quoted lines
> -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