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@xxxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |chkr@xxxxxxxxxxx --- Comment #1 from Christian Krause <chkr@xxxxxxxxxxx> 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. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review