Subject: Re: [PATCH/RFCv4 1/4] Documentation: Preparation for gitweb manpages

From: Jonathan Nieder <[email-masked]>
Date: Mon, 19 Sep 2011 18:37:03 -0500

Jakub Narebski wrote:

> Gitweb documentation currently consist of gitweb/README, gitweb/INSTALL
> and comments in gitweb source code.  This is harder to find, use and
> browse that manpages ("man gitweb" or "git help gitweb") and HTML
> documentation ("git help --web gitweb").

Language nits: s/consist/consists/; s/that manpages/than manpages/.  I
completely agree.

> The goal is to move documentation out of gitweb/README to gitweb.txt and
> gitweb.conf.txt manpages, reducing its size 10x from around 500 to
> around 50 lines (two pages), and move information not related drectly to
> building and installing gitweb out of gitweb/INSTALL there.

I guess you mean this patch prepares for or is part of a larger
project or series with that goal?  Wording nits: s/and move
information/and to move information/; s/drectly/directly/; s/ there//.
 
> The idea is to have gitweb manpage sources reside in AsciiDoc format
> in Documentation/ directory, like for gitk and git-gui.  Alternate
> solution would be to have gitweb documentation in gitweb/ directory,
> perhaps in POD format (see perlpod(1)).

Language nits: missing "the" before "Documentation/ directory" and
"gitweb manpage sources"; missing "An" before "Alternate solution".
I guess this is the most controversial aspect of the patch; your idea
seems sane enough to me.

> This patch adds infrastructure for easy generating gitweb-related
> manpages.  It adds currently empty 'gitweb-doc' target to
> Documentation/Makefile, and 'doc' proxy target to gitweb's Makefile.

Language nits: s/easy/easily/; missing "a" before "currently empty
'gitweb-doc' target" and "'doc' proxy target".

> This way to build gitweb documentation one can use
>
>   make -C gitweb doc
>
> or
>
>   cd gitweb; make doc

Language nit: a comma after "This way" would disambiguate.

Does "make -CDocumentation man html" build the gitweb documentation,
too (and "make install-doc" install it)?

[...]
> --- a/Documentation/Makefile
> +++ b/Documentation/Makefile
> @@ -170,6 +170,9 @@ info: git.info gitman.info
>  
>  pdf: user-manual.pdf
>  
> +GITWEB_DOC = $(filter gitweb.%,$(DOC_HTML) $(DOC_MAN1) $(DOC_MAN5) $(DOC_MAN7))
> +gitweb-doc: $(GITWEB_DOC)

Looks like no, alas.

Except for that detail, this looks good.

Thanks,
Jonathan