[Bug 452636] Review Request: mod_proxy_html - Module to rewrite content as it passes through an apache proxy.
bugzilla at redhat.com
bugzilla at redhat.com
Thu Aug 14 06:35:51 UTC 2008
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 #28 from Philip Prindeville <philipp at redfish-solutions.com> 2008-08-14 02:35:50 EDT ---
(In reply to comment #27)
> (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 ;)
That might be beyond the scope of this bug fix. ;-)
In any case, I've tried contacting the owners but not gotten any traction with
them.
> You'll need to work around that and version the tarball manually, I think this
> is covered in the wiki somewhere.
I looked at:
http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Non-Numeric_Version_in_Release
but didn't find it there. Nor is it in:
http://fedoraproject.org/wiki/Packaging/SourceURL
Can you point me at the right guidelines?
Also, rpmlint doesn't seem to have issue with Source0: not containing
%{version}. Is that a shortcoming of rpmlint?
> > > 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.
Not sure I follow you. Why does Apache support LoadFile then? Or is it a
Fedora policy to discourage using it? (It's not documented in:
http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Addon_Packages_.28httpd.2C_pam.2C_and_SDL.29
so where should I be looking?)
> 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.
More information about the package-review
mailing list