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@xxxxxxxxxxxxxxxxxxxxx> 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. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review