On 04/26/2014 01:59 AM, Amos Jeffries wrote:
> Also document HttpMsg::http_ver which is a copy of the HTTP-Version
> field in the "on-wire" message syntax. It is unrelated to the socket
> transport protocol and the URL scheme protocol.
I understand the above, but
> + /**
> + * The HTTP-Version label of this message.
> + */
> Http::ProtocolVersion http_ver;
the actual comment wording, especially its "label" part, sounds
confusing to me. Consider (quoting RFC 2616):
/// HTTP-Version field in the first line of the message.
> AnyP::UriScheme const & getScheme() const {return scheme_;}
>
> + /// convert the URL scheme to that given
> + void makeScheme(const AnyP::ProtocolType &p) {scheme_=p;}
> +
I would call this setScheme() for consistency with other
setters/getters, to preserve symmetry with the existing getScheme(), and
because the method does not really "make" schemes.
Alternatively, consider just making the scheme data member public and
deleting setters/getters. They are totally redundant right now can can
be added later if they become needed.
> - %PROTO Requested protocol
> + %PROTO Requested URL scheme (protocol)
I think the old description covered more cases because many URLs do not
have a scheme and some future FTP gatewayed requests may not even have
true URLs. How about something like:
%PROTO Requested origin server protocol (URL scheme)
None of the polishing touches above require another round of reviews as
far as I am concerned.
Thank you,
Alex.
