[Bug 452636] Review Request: mod_proxy_html - Module to rewrite content as it passes through an apache proxy.

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


https://bugzilla.redhat.com/show_bug.cgi?id=452636





--- Comment #27 from Joe Orton <jorton at redhat.com>  2008-08-13 07:23:21 EDT ---
(In reply to comment #26)
> (In reply to comment #22)
> > non-formal review:
> > 
> > 4) Source: http://apache.webthing.com/mod_proxy_html/mod_proxy_html.tgz
> > is bad - do upstream not provide versioned URLs?
> 
> Unfortunately, they do not.

upstream should be educated then ;)

You'll need to work around that and version the tarball manually, I think this
is covered in the wiki somewhere.

> > 5) using %{_sbindir}/apxs throughout is probably a good idea
> 
> Ok.  I thought that allowing for the potential of testing an alternate version
> of apxs might be a good thing.

I'm not sure it really makes a difference, but it reduces predictability if
apxs is picked from $PATH.

> All other comments have been addressed.

more nit-picking^W^Wreview:

1) this stuff is unnecessary obfuscation:

%define base proxy_html
%define modname mod_%{base}

Name: %{modname}

the spec file is for building mod_proxy_html, not an abstract httpd module; so
use "Name: mod_proxy_html" and hard-code proxy_html as necessary and
mod_proxy_html or %{name} otherwise.

2) the with-xml options should go; the module should be linked against -lxml2
and the conf file purged of LoadFile unconditionally.  The upstream method of
providing a deliberately broken module is totally crazy and not something that
should be supported (even as an option) in Fedora.

otherwise looks fine.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.