On Sun, Apr 28, 2013 at 03:26:10PM +0100, Justin B Rye wrote:
> Christian PERRIER wrote:
> > Your review should be sent as an answer to this mail.
> [...]

Thanks a bunch for this review! I'm excited that we make the debconf
messages/package description(s) better and more readable.

> >  Template: squid-deb-proxy/ppa-enable
> >  Type: boolean
> >  Default: false
> > -_Description: Allow PPA (Personal Package Archive) access?
> > - Squid-deb-proxy by default will not allow PPA repositories from launchpad.
> > - Selecting Y in this option will activate PPA repo access.
> > +_Description: Allow PPA access?
> > + By default, squid-deb-proxy does not allow to access Personal Packages
> > + Archive (PPA) repositories from Launchpad.
> 
> Disallowed!  "Allow" needs an object; here the simple fix would be to
> say "allow access to [...]".  But why are we using the word "allow" in
> the first place?  Installing squid-deb-proxy has no effect on whether
> I'm *permitted* to point my sources.list at a PPA; this debconf
> question just determines whether squid-deb-proxy will manage a cache
> to optimise package downloads from it.

The way squid-deb-proxy works is that it has a whitelist of
repositories it will connect to. If you connect to a different one it
will give you a "403 access denied". So in that sense its about
"permitting access", not only about caching (or the lack of
caching). 

You could of course enable the proxy for ftp.debian.org and disable it
for other source via apt.conf but that seems to be a corner-case.

The main use-case for this is that a admin can install it without any
configuration on the client and server and it will only allow access
to package sources that are considered official.
 
> It's "Personal Package Archive (PPA) repositories".  But I would avoid
> "access [...] from Launchpad" - make it "repositories on Launchpad".
> 
>     By default, squid-deb-proxy does not provide caching for Personal
>     Package Archive (PPA) repositories on Launchpad.

Thanks! I will fix this.

> > + .
> > + Choosing this option will allow this.
> 
> Again avoiding my least favourite word:
> 
>     Choosing this option will activate this support.
> 
> (It also helps smooth over the repeated "this".)

Thanks again, that sounds better indeed.

> >  Template: squid-deb-proxy/acl-disable
> >  Type: boolean
> >  Default: false
> >  _Description: Allow unrestricted network access?
> 
> Oh, but now this *does* mean "allow access".

So this part is fine?

> > + By default, squid-deb-proxy restricts access to the cache to private
> > + networks (10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16).
> > + .
> > + Choosing this option will allow other IP addresses to access the cache.
> > 
> > Reformulation to make things clearer (?).
> 
> I may be looking too hard for ambiguities, but I'm uneasy with
> "restricts access [...] to private networks".  I think I prefer:
> 
>     By default, squid-deb-proxy allows access to the cache from private
>     networks only (10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16).

I like your version better, its clearer what it means.

> On to the control file: the package description is all good idiomatic
> en_US, but I'm not entirely sure about the content.
> 
> > --- squid-deb-proxy.old/debian/control      2013-04-20 08:32:42.653002140 
> > +0200
> > +++ squid-deb-proxy/debian/control  2013-04-28 14:37:16.233346451 +0200
> > @@ -18,18 +18,15 @@
> >      squid3
> >  Recommends: avahi-utils
> >  Description: Squid proxy configuration optimized for deb packages
> > + This package contains a Squid proxy configuration that is optimized
> >   for downloading deb packages. It defaults to a different cache 
> > + directory and port than the regular Squid cache.
> 
> Do we need to specify "deb packages"?  This software is itself
> exclusively available in .deb format; nobody's reading this
> description to work out whether they need to install squid-deb-proxy
> or squid-rpm-proxy, so the synopsis doesn't really need to say.

+1
 
> And second, how is it "optimised for" .debs?  This strikes me as
> subtly backwards: Squid is only downloading and caching the files the
> way it always does, but the result is that it improves apt-get speeds.

Indeed, its not strictly about .deb packages you could use the config
for rpms as well (or just as a generic proxy). But there are some
tweaks specific for debs (like special handling of {,In}Release{,.gpg}
and some defaults like "maximum_object_size 512 MB" you wouldn't
normally use. So in this sense its optimized for .debs over the stock
squid config and its intended to be the "one-stop" place to put such
tweaks.

> My suggestion:
> 
>  Description: Squid proxy configuration to optimize package downloads
>   This package contains a Squid proxy configuration to manage a cache of
>   .deb package downloads, using a dedicated directory and port.
[..]

I would like to mention in some way that the config is different from
the stock config to accommodate for deb package. Not sure what the
best way for this is (its too early in the morning and I haven't had a
cup of tea yet ;)


I think this is looking good and once we clarify the remaining bits I
will be happy to upload the improved version to unstable. 


Cheers,
 Michael


-- 
To UNSUBSCRIBE, email to [email protected]
with a subject of "unsubscribe". Trouble? Contact [email protected]

Reply via email to