[Bug 230275] Review Request: varnish - High-performance HTTP accelerator

[bugzilla at redhat.com]

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





------- Additional Comments From ingvar at linpro.no  2007-04-23 17:18 EST -------
The short story:

Updated specfile: 
http://users.linpro.no/ingvar/varnish/fedora-extras-commit/varnish.spec

Updated source rpm:
http://users.linpro.no/ingvar/varnish/fedora-extras-commit/varnish-1.0.3-6.src.rpm


Details and comments:

* Matthias Saou
> - The %lib_name doesn't seem very useful, and having used plain
>   "libs" instead of "-n %{lib_name}" for the sub-package would make
>   things clearer. Also, the "future" devel package would be named
>   wrong since it would be "varnish-libs-devel".

The macro has been hanging around since I experimented with names and
an old mandrake-ish rpmlint. Agreed and fixed.

> - Some brackets are used inconsistently ("%version-%{release}").

Ah, thanks. Fixed.

> - A condrestart should probably be added in %postun, as it makes
>   sense to restart varnishd after an update.

Yup, added.

> - The .gz extensions in %files for the man pages are wrong, you should use
>   something like "*.1*" instead, for people who rebuild with uncompressed or
>   bzip2ed man pages.

Fixed

> - You could spare a lot of "mkdir -p" by using "install -D".

Fixed

> - The "--sbindir=/usr/sbin" on the %configure line is redundant.

No, it's not. At least, not unless this has been fixed in upstream
very, recently. In 1.0 it was needed.

> - The iteration for the UTF-8 conversion would be best done with a
>   glob, i.e.  "for i in bin/*/*.1", as it'll be less subject to break
>   if any programs are added or removed.

...and it became simpler and shorter too. Fixed.

> - I would personally add a comment above the "Requires: gcc" line to
>   explain that varnish *really* needs a C compiler at runtime by
>   design because of its VCL files.

Point. Fixed.

> - The explicit requirements on "ncurses" should be removed, as it's wrong to
>   have it (wouldn't allow for a compat-ncurses to work right).

Right. Removed, and let rpm find the correct deps by itself.

> - The kernel requirement should probably be removed from the libs
>   package, unless they are the ones requiring 2.6 specific features
>   (but I think it's only the daemon).

Yup, that's correct. Fixed.
 
> rpmlint's output is valuable, but having it empty unfortunately doesn't
> necessarily mean that the package is perfect!

But of course. Thanks for the input. I also stole some of your init-
and config file items :-)

Ingvar


-- 
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.