On Mon, Mar 25, 2019 at 10:28:21PM +0100, Johannes Schindelin wrote:

Hi,

I like the rest of the patch series, but this patch, I am not so sure
about...

On Sun, 24 Mar 2019, SZEDER Gábor wrote:

quoted

When our '.travis.yml' was split into several 'ci/*' scripts [1], the
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.

The big difference you introduce is that asciidoctor is now installed
with every job, not only with the Documentation job that actually uses it.

It is only installed in the 'Documentation' build job.

Even if it affects me very little (because I don't pay much attention to
Travis, it's been too flakey for me, and it does not test our Windows
side, and it is too slow), I'd rather install asciidoctor really only when
needed.

So I'd like to recommend to drop this patch from the series.

Thanks,
Dscho

quoted

[1] 657343a602 (travis-ci: move Travis CI code into dedicated scripts,
    2017-09-10)

Signed-off-by: SZEDER Gábor <redacted>
---
 ci/install-dependencies.sh | 3 +++
 ci/test-documentation.sh   | 3 ---
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index d64667fcbf..76ec308965 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh

@@ -54,6 +54,9 @@ StaticAnalysis)
 Documentation)

     ^^^^^^^^^^^^^^

quoted

 	sudo apt-get -q update
 	sudo apt-get -q -y install asciidoc xmlto
+
+	test -n "$ALREADY_HAVE_ASCIIDOCTOR" ||
+	gem install asciidoctor
 	;;
 esac

diff --git a/ci/test-documentation.sh b/ci/test-documentation.sh
index be3b7d376a..8f91f48c81 100755
--- a/ci/test-documentation.sh
+++ b/ci/test-documentation.sh

@@ -5,9 +5,6 @@

 . ${0%/*}/lib.sh

-test -n "$ALREADY_HAVE_ASCIIDOCTOR" ||
-gem install asciidoctor
-
 make check-builtins
 make check-docs

--

2.21.0.539.g07239c3a71.dirty