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

bugzilla at redhat.com bugzilla at redhat.com
Sat Jun 28 23:09:57 UTC 2008


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:http-mod_proxy_html - Module to rewrite content as it passes through an apache proxy.


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





------- Additional Comments From rayvd at bludgeon.org  2008-06-28 19:09 EST -------
(In reply to comment #3)
> I have no problem merging them.  That's fine.
> 
> Some observations:
> 
> * we don't need -Wl,"-lxml2" in the invocation of apxs.  We can just use -lxml2 
> directly;

I'm definitely not an expert on apxs, does  -lxml2 get added automatically
somewhere later in the process?  I originally did not use it and resulted in a
.so that wasn't linked against libxml2.so.  Had to use LoadFile in Apache to get
things working.
 
> * similarly, it would be preferable to install via "apxs -i -S 
> LIBEXECDIR=$RPM_BUILD_ROOT/%{modulesdir} -n %{modname} %{modname}.la" as I've 
> done; the .libs directory is an implementation detail of apxs using libtool 
> that we shouldn't rely on;

That makes sense to me!

> Do we really need to supply the path to apxs?  It should be in the default 
> search path.  If it's not, it might be because someone wants to try out a new 
> version of it (in which case we should use that anyway).

I don't think we do, I just typically prefer to be more specific than less.

> Other than that, I'm fine with the rest of the changes.
> 

I'll merge in your suggestions to my spec file.  I don't have a burden to be the
maintainer particularly if you are interested, just let me know.

-- 
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, or are watching someone who is.




More information about the Fedora-package-review mailing list