Re: [PATCH v3 0/6] Asciidoctor-related formatting and CI fixes

From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
To: "SZEDER Gábor" <szeder.dev@gmail.com>
Cc: "Junio C Hamano" <gitster@pobox.com>,
	git@vger.kernel.org, "Jeff King" <peff@peff.net>,
	"Todd Zullinger" <tmz@pobox.com>,
	"Martin Ågren" <martin.agren@gmail.com>,
	"brian m. carlson" <sandals@crustytoothpaste.net>
Subject: Re: [PATCH v3 0/6] Asciidoctor-related formatting and CI fixes
Date: Fri, 29 Mar 2019 16:56:28 +0100 (STD)	[thread overview]
Message-ID: <nycvar.QRO.7.76.6.1903291653590.41@tvgsbejvaqbjf.bet> (raw)
In-Reply-To: <20190329123520.27549-1-szeder.dev@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 3058 bytes --]

Hi,

On Fri, 29 Mar 2019, SZEDER Gábor wrote:

> [...]
>   - Mention in patch 4's commit message that Asciidoctor is still only
>     installed in the Documentation build job.
> [...]
>
> Range-diff:
> 1:  8026f62876 = 1:  8026f62876 Documentation/git-diff-tree.txt: fix formatting
> 2:  fd19cf4b24 = 2:  fd19cf4b24 Documentation/technical/api-config.txt: fix formatting
> 3:  638dcd64e9 = 3:  638dcd64e9 Documentation/technical/protocol-v2.txt: fix formatting
> 4:  6f8c6ff398 ! 4:  2e94e2b7b6 ci: install Asciidoctor in 'ci/install-dependencies.sh'
>     @@ -6,8 +6,9 @@
>          installation of the 'asciidoctor' gem somehow ended up in
>          'ci/test-documentation.sh'.
>
>     -    Install it in 'ci/install-dependencies.sh', where we install
>     -    everything else.
>     +    Install it in 'ci/install-dependencies.sh', where we install other
>     +    dependencies of the Documentation build job as well (asciidoc,
>     +    xmlto).

I would have wished for something that more explicitly said that it still
*only* installs it for the Documentation job.

But I can live with the current wording.

>          [1] 657343a602 (travis-ci: move Travis CI code into dedicated scripts,
>              2017-09-10)
> [...]
>
>     @@ -66,15 +77,32 @@
>       +++ b/ci/test-documentation.sh
>      @@
>
>     + . ${0%/*}/lib.sh
>     +
>     ++filter_log () {
>     ++	sed -e '/^GIT_VERSION = /d' \
>     ++	    -e '/^    \* new asciidoc flags$/d' \
>     ++	    "$1"
>     ++}
>     ++
>     + make check-builtins
>     + make check-docs
>     +
>       # Build docs with AsciiDoc
>     - make doc > >(tee stdout.log) 2> >(tee stderr.log >&2)
>     +-make doc > >(tee stdout.log) 2> >(tee stderr.log >&2)
>      -! test -s stderr.log
>     -+cat stderr.log
>     ++make doc > >(tee stdout.log) 2> >(tee stderr.raw >&2)
>     ++cat stderr.raw
>     ++filter_log stderr.raw >stderr.log
>      +test ! -s stderr.log
>       test -s Documentation/git.html
>       test -s Documentation/git.xml
>       test -s Documentation/git.1
>     -@@
>     + grep '<meta name="generator" content="AsciiDoc ' Documentation/git.html
>     +
>     +-rm -f stdout.log stderr.log
>     ++rm -f stdout.log stderr.log stderr.raw
>     + check_unignored_build_artifacts
>
>       # Build docs with AsciiDoctor
>       make clean
>     @@ -82,8 +110,8 @@
>      -sed '/^GIT_VERSION = / d' stderr.log
>      -! test -s stderr.log
>      +make USE_ASCIIDOCTOR=1 doc > >(tee stdout.log) 2> >(tee stderr.raw >&2)
>     -+sed '/^GIT_VERSION = / d' stderr.raw >stderr.log
>     -+cat stderr.log
>     ++cat stderr.raw
>     ++filter_log stderr.raw >stderr.log
>      +test ! -s stderr.log
>       test -s Documentation/git.html
>       grep '<meta name="generator" content="Asciidoctor ' Documentation/git.html

Wow. Subtle. And a bit hard to read without color ;-) But from what I
understand, this does the right thing.

So from my side, this patch series is good to go!

Thanks,
Dscho