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=744977 --- Comment #6 from Mathieu Bridon <bochecha@xxxxxxxxxxxxxxxxx> 2012-01-26 04:18:49 EST --- First of all, I want to apologize for taking so long to answer. It seems that this review was part of Matthieu's sponsoring process and I hope my failure to react in a timely fashion didn't have any negative consequence on it, either for you, Matthieu, or for your sponsor, Martin. (In reply to comment #3) > Issues: > [!]: MUST Each %files section contains %defattr if rpm < 4.4 > Note: defattr(....) present in %files devel section. This is OK if > packaging for EPEL5. Otherwise not needed Thanks, I removed the %defattr lines. > [!]: MUST Package requires pkgconfig, if .pc files are present. (EPEL5) > Note: Only applicable for EL-5 I'll ignore this since I'm not targeting EPEL 5. (In reply to comment #4) > There are some things that should be addressed before the package is checked > in: > > - the devel package should require the base package this way: > http://fedoraproject.org/wiki/PackagingGuidelines#Requiring_Base_Package Thanks, I somehow missed specifying the architecture. > - Don't add the %doc files several times. Drop AUTHORS, LICENSE, and COPYING > from the devel package. Since it requires the base package, these files are > installed anyway. Right, I fixed that. > - add README and NOTICE to the base package (with %doc) and doc/QUICK_START to > the devel package Good catch, I added those. > - I suggest to build the doxygen API documentation (cd into docs/ and run > doxygen doxygen.conf) the devel package. Done, but since the generated doc is rather large I've added it to a -doc subpackage (noarch). > - Either add a Group field to the base package (System Environment/Libraries), > or remove it from the devel package. Currently, the Group field is used > inconsistently. I had explicitly removed the one on the base package, but somehow forgot to do that for the devel subpackage as well. This is fixed. ---- I also updated to the latest upstream VCS snapshot, as it brings in a couple of bug fixes, better unit testing, and it makes it easier to build the doxygen documentation. Spec URL: http://bochecha.fedorapeople.org/packages/libhtp.spec SRPM URL: http://bochecha.fedorapeople.org/packages/libhtp-0.3.0-0.3.20120126.git53e5901.fc16.src.rpm Matthieu, were you already sponsored at the time you approved the package? Martin, since you had a few issues with the approved package, can I consider the review granted and ask for the SCM branches? Thanks, and once again please accept my apologies for delaying the review for such a long time. -- 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. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review