On Wed, May 04, 2016 at 02:34:23PM -0700, Junio C Hamano wrote:
> Third time's a charm, perhaps?
>
> -- >8 --
> Subject: [PATCH] ci: validate "gitlink:" in documentation
>
> It is easy to add incorrect "linkgit:<page>[<section>]" references
> to our documentation suite. Catch these common classes of errors:
>
> * Referring to Documentation/<page>.txt that does not exist.
>
> * Referring to a <page> outside the Git suite. In general, <page>
> must begin with "git".
>
> * Listing the manual <section> incorrectly. The first line of the
> Documentation/<page>.txt must end with "(<section>)".
>
> with a new script "ci/lint-gitlink", and drive it from "make check-docs".
>
> Signed-off-by: Junio C Hamano <[email protected]>
> ---
This looks good to me. Two minor nits that may not be worth addressing:
> +lint-docs::
> + $(QUIET_LINT)$(foreach txt,$(patsubst %.html,%.txt,$(DOC_HTML)), \
> + ../ci/lint-gitlink $(txt))
> +
This dependency feels funny. Wouldn't CI want to invoke this as:
make -C Documentation lint-docs
IOW, Documentation owns the script (just like t/ owns its own lint
scripts like check-non-portable-shell.pl), and CI always just triggers
the make-driven checks, just as a normal developer would?
> +sub lint {
> + my ($file) = @_;
> + open my $fh, "<", $file
> + or return;
> + while (<$fh>) {
> + my $where = "$file:$.";
> [... actual linting of line ...]
> +}
> +
> +for (@ARGV) {
> + lint($_);
> +}
The usual perl way here would be:
while(<>) {
my $where = "$ARGV:$.";
... actual linting of line ...
}
where "<>" automagically reads from files on the command-line or stdin
if none were specified (and sets $ARGV as appropriate). But maybe you
prefer not to handle the stdin case (it is a benefit sometimes, but an
annoyance if you accidentally end up with an empty file list).
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
