[Tarantool-patches] [PATCH v4 1/4] http: add CURLOPT_ACCEPT_ENCODING option

Alexander Turenko alexander.turenko at tarantool.org
Tue Oct 29 02:42:17 MSK 2019


I have only a few comments about texts, the code is okay.

WBR, Alexander Turenko.

> http: add CURLOPT_ACCEPT_ENCODING option

I would use 'httpc:' prefix for http client commits: we did it for
several recent patches about http client.

I would say that accept_encoding option was added or that
CURLOPT_ACCEPT_ENCODING binding is added.

>
> CURLOPT_ACCEPT_ENCODING option is now supported.
> This option enables automatic decompression of HTTP downloads.
> See https://curl.haxx.se/libcurl/c/CURLOPT_ACCEPT_ENCODING.html
> 
> Closes #4232
> 
> @TarantoolBot document
> Title: http: CURLOPT_ACCEPT_ENCODING option

The same comments as above are applicable here too.

> Update the documentation for curl options to reflect new
> CURLOPT_ACCEPT_ENCODING option. It enables automatic

And here.

> decompression of HTTP downloads by setting the contents of the
> Accept-Encoding header sent in a HTTP request and enabling
> decoding of a response when a Content-Encoding header is received.
> This is a request, not an order; the server may or may not do it.
> Servers might respond with Content-Encoding even without getting
> an Accept-Encoding in the request. Servers might respond with a
> different Content-Encoding than what was asked for in the request.
> @param encoding specifies what encoding you'd like. This param

Doxygen-style @param here is a typo? The public http client API only has
'accept_encoding' option, not 'encoding'.

> can be an empty string which means Accept-Encoding header will
> contain all built-in supported encodings. This param can be
> comma-separated list of accepted encodings, like:
> "br, gzip, deflate". Bundled libcurl supports "identity",
> meaning non-compressed, "deflate" which requests the server to
> compress its response using the zlib algorithm and "gzip" which
> requests the gzip algorithm. System libcurl also possibly
> supports "br" which is brotli.
> See https://curl.haxx.se/libcurl/c/CURLOPT_ACCEPT_ENCODING.html

I suggest to split this block of text by paragraph to ease reading.

> +/**
> + * Enable automatic decompression of HTTP downloads: set the
> + * contents of the Accept-Encoding header sent in a HTTP request
> + * and enable decoding of a response when a Content-Encoding
> + * header is received. This is a request, not an order; the
> + * server may or may not do it. Servers might respond with
> + * Content-Encoding even without getting an Accept-Encoding in the
> + * request. Servers might respond with a different
> + * Content-Encoding than what was asked for in the request.
> + * @param req request
> + * @param encoding - specify what encoding you'd like. This param
> + * can be an empty string which means Accept-Encoding header will
> + * contain all built-in supported encodings. This param can be
> + * comma-separated list of accepted encodings, like:
> + * "br, gzip, deflate". Bundled libcurl supports "identity",
> + * meaning non-compressed, "deflate" which requests the server to
> + * compress its response using the zlib algorithm and "gzip" which
> + * requests the gzip algorithm. System libcurl also possibly
> + * supports "br" which is brotli.
> + * @see https://curl.haxx.se/libcurl/c/CURLOPT_ACCEPT_ENCODING.html

The same: I would split it to meaningful paragraphs to ease reading.


More information about the Tarantool-patches mailing list