1002775 - For the purpose of continuous integration testing, makedocs.pl should return failed status if one or more errors occurred building docs

Status: RESOLVED FIXED

(Bugzilla :: Documentation, enhancement)

Description

[David Lawrence [:dkl]]

Currently if docs/makedocs.pl fails in any of the steps it returns a 0 since the return value for each of the system() calls is not inspected. We should check the return value and die if any part of the docs process fails. This will also allow proper failure detection in the Travis CI system I am working on.

Patch coming.
dkl

Comment 1

[Frédéric Buclin]

We shouldn't die. We should continue with compiling the remaining doc.

Comment 2

[David Lawrence [:dkl]]

(In reply to Frédéric Buclin from comment #1)
> We shouldn't die. We should continue with compiling the remaining doc.

Then I will check the return value of each system() call, collect the results and at the end return the correct result if there were one or more errors. This will still satisfy what I need.

dkl

Comment 3

[David Lawrence [:dkl]]

Attachment: 1002775_1.patch

Comment 4

[:glob ✱]

Comment on attachment 8414494 [details] [diff] [review]
1002775_1.patch

Review of attachment 8414494 [details] [diff] [review]:
-----------------------------------------------------------------

::: docs/makedocs.pl
@@ +63,5 @@
>  
>      say "Creating $name documentation ..." if defined $name;
>      say "$cmdline\n";
>      system $cmdline;
> +    $error_found = 1 if $?;

you should be checking value returned by system() instead:
system($cmdline) == 0
    or $error_found = 1;

Comment 5

[David Lawrence [:dkl]]

Attachment: 1002775_2.patch

Comment 6

[:glob ✱]

Comment on attachment 8414502 [details] [diff] [review]
1002775_2.patch

Review of attachment 8414502 [details] [diff] [review]:
-----------------------------------------------------------------

r=glob

Comment 7

[David Lawrence [:dkl]]

To ssh://[e-mail]/bugzilla/bugzilla.git
   cfbeadc..4ad3e58  master -> master

Comment 8

[Frédéric Buclin]

Comment on attachment 8414502 [details] [diff] [review]
1002775_2.patch

>+die "Error occurred building the documentation\n" if $error_found;

I don't see the point to die at this point. You could simply return $error_found.

Comment 9

[Frédéric Buclin]

Reopening, as I think it's wrong to die, and this doesn't bring any value.

Comment 10

[David Lawrence [:dkl]]

Attachment: 1002775_3.patch

Comment 11

[David Lawrence [:dkl]]

Attachment: 1002775_4.patch

Comment 12

[:glob ✱]

Comment on attachment 8414722 [details] [diff] [review]
1002775_4.patch

(In reply to Frédéric Buclin from comment #9)
> Reopening, as I think it's wrong to die, and this doesn't bring any value.

i disagree, which is why this patch got my approval+

provided a clear final error when something went wrong while building documentation is useful.