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@xxxxxxxxx 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. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review