[Bug 483434] Review Request: argtable2 - A library for parsing GNU style command line arguments

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=483434


Christian Krause <chkr at plauener.de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |chkr at plauener.de




--- Comment #1 from Christian Krause <chkr at plauener.de>  2009-02-16 18:31:57 EDT ---
This is just an informal review - I hope it helps anyway:

build test:
- package builds fine on F10 and F11 for all architectures


major issue:
- md5sum of argtable2-10.tar.gz contained in the src.rpm doesn't match the
upstream package


minor issues:

- I don't know whether there are strict rules regarding the documentation, but
I would rather put the content of /usr/share/doc/argtable2 into
/usr/share/doc/argtable2-devel-10, because the documentation seems to be
necessary only for development purposes.

- according to
http://fedoraproject.org/wiki/Packaging/SourceURL#Sourceforge.net Source0
should be http://downloads.sourceforge.net/argtable/%{name}-%{version}.tar.gz
(and not prdownloads.sf.net)

- rpmlint:
rpmlint SRPMS/argtable2-10-1.fc10.src.rpm RPMS/i386/argtable2-*
SPECS/argtable2.spec
argtable2-devel.i386: W: hidden-file-or-dir
/usr/share/doc/argtable2-devel-10/tests/.deps
argtable2-devel.i386: W: hidden-file-or-dir
/usr/share/doc/argtable2-devel-10/tests/.deps

I've had a look at this tests directory and since it was copied out of an
source tree which uses auto* tools it cannot be used easily ouside. E.g. copy
the directory and try a make fails:

make: *** No rule to make target `../configure.ac', needed by `Makefile.in'. 
Stop.

Additionally it looks like that the tests really work only when built from
within the source, since they use includes like this:
fntests.c:
#include "../src/argtable2.h"
#include <assert.h>

Since the tests cannot be compiled just with the installed header files of the
library anyway, probably it would be better not to package them at all.

argtable2-devel.i386: W: spurious-executable-perm
/usr/share/doc/argtable2-devel-10/tests/fntests.sh
argtable2-devel.i386: W: spurious-executable-perm
/usr/share/doc/argtable2-devel-10/tests/test_dbl.sh
argtable2-devel.i386: W: spurious-executable-perm
/usr/share/doc/argtable2-devel-10/tests/test_int.sh
argtable2-devel.i386: W: spurious-executable-perm
/usr/share/doc/argtable2-devel-10/tests/test_date.sh
argtable2-devel.i386: W: spurious-executable-perm
/usr/share/doc/argtable2-devel-10/tests/test_rex.sh
argtable2-devel.i386: W: spurious-executable-perm
/usr/share/doc/argtable2-devel-10/tests/test_lit.sh
4 packages and 1 specfiles checked; 0 errors, 8 warnings.

Since these are shell scripts it should be OK to that they are executable,
however I suggest not to package the tests at all.

Instead of "tests" it would be an option to package "examples" into
/usr/share/doc/argtable2-devel-10/: "examples" contains a bunch of good
examples and can be compiled (even outside of the source tree) easily.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.