Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Review Request: varnish - High-performance HTTP accelerator https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=230275 bugzilla@xxxxxxxxxx changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|normal |medium ------- Additional Comments From ingvar@xxxxxxxxx 2007-04-17 14:59 EST ------- The short story: New version of the specfile: http://users.linpro.no/ingvar/varnish/fedora-extras-commit/varnish.spec New source rpm: http://users.linpro.no/ingvar/varnish/fedora-extras-commit/varnish-1.0.3-5.src.rpm The details: * Kevin Fenzi > 1. You might want to use the %{?dist} tag. OK, added the dist tag in Relase > 2. You should not strip the binaries and libraries... > that makes the rpm debuginfo > package empty. Remove the 'strip -p' lines in install. > rpmlint says: > E: varnish-debuginfo empty-debuginfo-package Ah, I had a %debug_package %{nil} in my .rpmmacros, som I didn't catch this one with rpmlint. Fixed. > 3. Don't use $RPM_BUILD_ROOT and %{buildroot}. OK, fixed. > 4. You might use for the Source0 url something like: > http://downloads.sourceforge.net/varnish/varnish-%{version}.tar.gz > Not a blocker, but nice to not point to a specific mirror. OK, fixed. > 5. You shouldn't need to specify Buildroot for each of the > subpackages. It should pick that up from the top. I think the > comment above was about "BuildRequires", not "Buildroot". Also, no > need for another URL tag for the subpackages. OK, fixed > 6. Do you need to ship static libs? They are discouraged in Fedora > for a variety of reasons. Do you know anything that links to just > the static libs? Or can we remove them for now and readd a -static > subpackage later if someone requests? Yep, removed. > 7. Subpackages shouldn't also need to duplicate all the doc files: > %doc INSTALL LICENSE README ChangeLog redhat/README.redhat OK, added only the LICENCE file to avoid the "no documentation" warning from rpmlint. > Perhaps they should just be in the main package since the others require it? Yep, that sounds reasonable > Also, no need to include the INSTALL file, since people reading here will be > installing via the rpm. ;) That is not necessarily true. The INSTALL file may contain information which could inspire users to compile a version of varnish on their own. Which is not a bad thing. I kept the INSTALL file. > 8. Does the description in the main package need to have all the > links and copyright info? It's just a dump of the README, so no problem in trimming it down a bit. > 9. Should remove the > /sbin/chkconfig --list varnish > in %post. rpms shouldn't have any output when installing. Also > missing some requires for the init script handling, suggest: > > Requires(post): /sbin/chkconfig > Requires(preun): /sbin/chkconfig > Requires(preun): /sbin/service OK, fixed as proposed. > 10. You need to own the /etc/varnish directory... > %dir %{_sysconfdir}/varnish/ OK, fixed -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug, or are watching the QA contact. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review