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@fedoraproject.org 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.git53e59...
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.