[Bug 230275] Review Request: varnish - High-performance HTTP accelerator
bugzilla at redhat.com
bugzilla at redhat.com
Tue Apr 17 19:00:07 UTC 2007
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 at redhat.com changed:
What |Removed |Added
----------------------------------------------------------------------------
Severity|normal |medium
------- Additional Comments From ingvar at linpro.no 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.
More information about the Fedora-package-review
mailing list