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@plauener.de changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |chkr@plauener.de
--- Comment #1 from Christian Krause chkr@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/%%7Bname%7D-%%7Bversion%7D.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.