You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Luca Toscano <el...@apache.org> on 2016/12/30 18:47:15 UTC
Re: svn commit: r1776616 - in /httpd/httpd/trunk/docs/manual/mod:
mod_remoteip.html.en mod_remoteip.xml.fr mod_remoteip.xml.meta
Hi Daniel and Jim,
I saw your comments in one of the last email thread about Daniel's new code
change for mod_remoteip, and I have some questions for the doc about the
naming of the new directive:
2016-12-30 19:20 GMT+01:00 <el...@apache.org>:
> Author: elukey
> Date: Fri Dec 30 18:20:04 2016
> New Revision: 1776616
>
> URL: http://svn.apache.org/viewvc?rev=1776616&view=rev
> Log:
> Documentation rebuild for mod_remoteip
>
> Modified:
> httpd/httpd/trunk/docs/manual/mod/mod_remoteip.html.en
> httpd/httpd/trunk/docs/manual/mod/mod_remoteip.xml.fr
> httpd/httpd/trunk/docs/manual/mod/mod_remoteip.xml.meta
>
> + using the <code class="directive"><a href="#
> remoteipproxyprotocolenable">RemoteIPProxyProtocolEnable</a>
> +<div class="directive-section"><h2><a name="RemoteIPProxyProtocol"
> id="RemoteIPProxyProtocol">RemoteIPProxyProtocol</a> <a
> name="remoteipproxyprotocol" id="remoteipproxyprotocol">Directive</a></h2>
>
> +<tr><th><a href="directive-dict.html#Syntax">Syntax:</a></th><td><code>ProxyProtocol
> On|Optional|Off</code></td></tr>
In the above snippets I can see three different names:
RemoteIPProxyProtocolEnable, RemoteIPProxyProtocol and ProxyProtocol. The
new directive in the C code is called RemoteIPProxyProtocolEnable, but I
can see new logs using also RemoteIPProxyProtocol (like
"RemoteIPProxyProtocol: internal error: have data left over; ").
I personally like a lot RemoteIPProxyProtocol (rather than
RemoteIPProxyProtocolEnable that seems a bit heavy to read), but everything
is fine as long as we use a single name, especially in the logs that admins
will read :)
I haven't checked the code in detail so I might say something completely
irrelevant, just writing the first things that I noticed!
Luca
Re: svn commit: r1776616 - in /httpd/httpd/trunk/docs/manual/mod:
mod_remoteip.html.en mod_remoteip.xml.fr mod_remoteip.xml.meta
Posted by Luca Toscano <to...@gmail.com>.
2016-12-30 20:07 GMT+01:00 Daniel Ruggeri <DR...@primary.net>:
> On 12/30/2016 12:47 PM, Luca Toscano wrote:
> > I personally like a lot RemoteIPProxyProtocol (rather than
> > RemoteIPProxyProtocolEnable that seems a bit heavy to read), but
> > everything is fine as long as we use a single name, especially in the
> > logs that admins will read :)
> >
> > I haven't checked the code in detail so I might say something
> > completely irrelevant, just writing the first things that I noticed!
>
> Hrm - yes... these are inconsistencies that came up as I was renaming
> stuff and moving it around. I also like the suggestion to shorten the
> name since "Enable" is repetitive... I have adjusted the references to
> be RemoteIPProxyProtocol in docs and code, fixed references to the
> "PROXY protocol" to align with the case that HAProxy uses as well as
> removed the mod_proxy_protocol module in r1776624. Thanks for the pointer.
Just updated the mod_remoteip doc after your last commit, it looks really
nice now! Thanks a lot!
Luca
Re: svn commit: r1776616 - in /httpd/httpd/trunk/docs/manual/mod:
mod_remoteip.html.en mod_remoteip.xml.fr mod_remoteip.xml.meta
Posted by Daniel Ruggeri <DR...@primary.net>.
On 12/30/2016 12:47 PM, Luca Toscano wrote:
> I personally like a lot RemoteIPProxyProtocol (rather than
> RemoteIPProxyProtocolEnable that seems a bit heavy to read), but
> everything is fine as long as we use a single name, especially in the
> logs that admins will read :)
>
> I haven't checked the code in detail so I might say something
> completely irrelevant, just writing the first things that I noticed!
Hrm - yes... these are inconsistencies that came up as I was renaming
stuff and moving it around. I also like the suggestion to shorten the
name since "Enable" is repetitive... I have adjusted the references to
be RemoteIPProxyProtocol in docs and code, fixed references to the
"PROXY protocol" to align with the case that HAProxy uses as well as
removed the mod_proxy_protocol module in r1776624. Thanks for the pointer.
--
Daniel Ruggeri