[Bug 165689] Review Request: SquidGuard: filter, redirector and access controller plugin for squid
bugzilla at redhat.com
bugzilla at redhat.com
Mon Sep 5 08:24:02 UTC 2005
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: SquidGuard: filter, redirector and access controller plugin for squid
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=165689
------- Additional Comments From oliver at linux-kernel.at 2005-09-05 04:24 EST -------
Thanks for your review!
(In reply to comment #1)
> * the logrotate file contains "olddir /var/log/old/squid". This is not standard,
> please remove it.
Removed.
> * this file should also be tagged as %config(noreplace) in case the user makes
> changes.
Done.
> * in the package's description, there are "**)" symbols. What does that mean ?
> Maybe a bad copy/paste ?
Of course - removed.
> * Source3 is guard-distrib.tar.gz: where does it come from ? It is copied to
> samples/, but not included in the rpm (not in the %files section). If it is not
> useful, please remove it.
Removed.
> * SMP flags are not used. If it does not build with it, please write a small
> comment above the make command
Added smpmflags
> * the checks "RPM_BUILD_ROOT != /" are useless in %install and %clean, please
> remove them.
Remoevd.
> * use install -p to preserve timestamps
OK - Done.
> * the "mkdir -p $RPM_BUILD_ROOT%{_dbhomedir}" line in %install is useless, the
> dir was created with install above
Removed.
> * "%dir %{_dbhomedir}" and "%{_dbhomedir}/*" can be summed up in simply
> "%{_dbhomedir}/"
Changed.
> * don't create $RPM_BUILD_ROOT%{_datadir}/%{name} in %install, since there are
> no files in it.
Removed.
> * Source 2 is not available anymore. In the same directory, there is a
> blacklists.tar.gz file which seems to be the same as the versionned one. Maybe
> you could use that.
Changed. There should be always a timestamped one and one without timestamp,
which is the same. I removed the _bldate macro and use the non-timestamped one now.
> * BuildRoot: should be
> %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
Yes, it should...
> * The package should contain the text of the license. Please include the COPYING
> file.
Done. Shall I also include the GPL file from the tarball?
> * The default squidGuard.conf file does not work since your blacklists are in
> %dbhomedir/blacklists and the file refers to %dbhomedir/dest. It would be nice
> to make it work by default, and a simple "s,dest/adult/,blacklists/porn/,g" on
> the file should fix it.
Fixed with the provided regex. I didn't fix it, because I thought, that the
admin who installs it, will have a look at the config...
> There is still the problem of the invalid redirection,
> but I don't think we can do much about that, can we ?
I would have to install the guard.cgi and guard.conf into some place and create
an Apache config. But we'll never know the hostname/FQDN of the host running
squidGuard. It could also be possible, that the admin has no webserver on the
proxy-machine and want's to install the cgi on another server. So I think we
_can_ do something about that, but it doesn't make much sense... :-(
> * a possible improvement would be to add a script to update the blacklists. Many
> are available on the web, you could for example try this one:
> http://cuda.port-aransas.k12.tx.us/squid-getlist.html or base yours on it. This
> is not a blocker, but could be a nice improvement if you are interested in
> squidGuard.
Added this and slightly modified it (paths). I also added a var called ENABLED,
which is 0 (therfor disabled) by default...
Build 8. Please have a look.
--
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
More information about the fedora-extras-list
mailing list