Re: [PATCH 2/3] Add gitmodules(5)

On 6/10/07, Frank Lichtenheld [off-list ref] wrote:

The unofficial git documentation nitpicker at work ;)

That's good, keep it up :)

On Sat, Jun 09, 2007 at 11:38:51PM +0200, Lars Hjemli wrote:

quoted

+The `.gitmodules` file, located in the top-level directory of a
+gitlink:git[7] working tree, is a text file with a layout matching the

That link seems superfluous to me.

Yeah, what I really wanted here was a link to the definition of "git
working tree". I'll drop it.

I would have used "syntax" instead of "layout".

Agreed

quoted

+requirements of gitlink:git-config[1].
+
+The file consists of sections named `module`, divided into one subsection
+per submodule. The subsections are named with the logical name of the
+submodule it describes.

"sections named module" sounds confusing to me. Why are there multiple
sections named module? (for the record: I know what you mean, I just
don't know if it couldn't be said simpler)
Maybe better "subsections of section `module`, one per submodule"?
Hmm, sounds ugly too.

Good documentation is hard, so I'll work on it some more...

quoted

+Each submodule can contain the following keys.
+
+module.$name.path::
+     Define a path, relative to the top-level directory of the git
+     working tree, where the submodule is expected to be checked out.
+
+module.$name.url::
+     Define a url from where the submodule repository can be cloned.

For .path a "Defaults to name of submodule" probably wouldn't hurt.

True. But there might be some issues with this rule, so I'll leave it
as is for now.

For the sake of documentation consistency I would suggest
module.<name>.path. You can compare the output of
$ grep "\.<[a-z]" Documentation/*.txt
with
$ grep "\.\$[a-z]" Documentation/*.txt
to see what I mean.

That was very descriptive, thanks!

quoted

+     [module 'libfoo']
+             path = include/foo
+             url = git://example1.com/git/libfoo.git
+
+     [module 'libbar']
+             url = git://example2.com/pub/git/libbar.git

This would actually be a syntax error in a git config file
(subsection names can be enclosed in "" but not '').

Shame on me for drinking while documenting ;-)

Thanks for the review, I'll try to send a fixed-up patch later today.

--
larsh