Re: [LTP] [PATCH 1/1] configure.ac: Fix summary for disabled metadata

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> > Don't we stil have the same problem with "$enable_metadata_html" and
> > "$enable_metadata_pdf" ?
> Well, this patch was about configure output. I wouldn't print them at all if
> metadata disabled. They're not used anyway in the code if metadata disabled.

I guess that I'm still confused by the configure script, because we do
use enable_metadata_html and enable_metadata_pdf but I do not get how
the enable_metadata works, we do not seem to use it for anything but
perl module check in there.

What I would have expected there would be:

diff --git a/m4/ltp-docparse.m4 b/m4/ltp-docparse.m4
index 88d2e08e4..6f0bef1c9 100644
--- a/m4/ltp-docparse.m4
+++ b/m4/ltp-docparse.m4
@@ -35,7 +35,12 @@ with_metadata=no
 with_metadata_html=no
 with_metadata_pdf=no

-if test "x$enable_metadata" = xyes && test "x$enable_metadata_html" = xyes -o "x$enable_metadata_pdf" = xyes; then
+if test "x$enable_metadata" != xyes; then
+       enable_metadata_html=no
+       enable_metadata_pdf=no
+fi
+
+if test "x$enable_metadata_html" = xyes -o "x$enable_metadata_pdf" = xyes; then
        AX_PROG_PERL_MODULES(Cwd File::Basename JSON LWP::Simple)
 fi

And that would cause both with_metadata_html and with_metadata_pdf to be
set to 'no' and the configure summary would be correct to begin with.

> > I think that we should rethink what the flags really do, I guess that
> > for instance it would make sense for the $enable_metadata=no to just set
> > both $enable_metadata_html and $enable_metadata_pdf to no and the rest
> > of the m4/ltp-docparse.m4 should just check the later two.
> Rethink the flags meaning or a functionality?
> 
> And how about flags names? What name would you suggest?
> --disable-doc, --with-html-doc, --with-pdf-doc ?
> Because we now have metadata (i.e. JSON output) mandatory.

I guess that I would call this 'autodoc' or 'docparse' or something
that describes that it's generated documentation.

> I wonder what is needed to be fixed now and what's better to postpone after the
> release?

Depends on the size of the patch, if it's small enough, like the one I
posted above it should be okay.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp