From: Alexander Turenko <alexander.turenko@tarantool.org> To: Ilya Kosarev <i.kosarev@tarantool.org> Cc: tarantool-patches@freelists.org, tarantool-patches@dev.tarantool.org Subject: Re: [Tarantool-patches] [PATCH v4 1/4] http: add CURLOPT_ACCEPT_ENCODING option Date: Tue, 29 Oct 2019 02:42:17 +0300 [thread overview] Message-ID: <20191028234216.akbicvptcgov2j3x@tkn_work_nb> (raw) In-Reply-To: <ba1c2c99566a1f0c59a1e5d1913c871a94a1dc6e.1572282336.git.i.kosarev@tarantool.org> 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.
next prev parent reply other threads:[~2019-10-28 23:42 UTC|newest] Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-10-28 17:11 [Tarantool-patches] [PATCH v4 0/4] http: add CURLOPT_ACCEPT_ENCODING option and following improvements Ilya Kosarev 2019-10-28 17:11 ` [Tarantool-patches] [PATCH v4 1/4] http: add CURLOPT_ACCEPT_ENCODING option Ilya Kosarev 2019-10-28 23:42 ` Alexander Turenko [this message] 2019-10-28 17:11 ` [Tarantool-patches] [PATCH v4 2/4] http: remove redundant & incorrect case for curl_request code Ilya Kosarev 2019-10-28 23:42 ` Alexander Turenko 2019-10-28 17:11 ` [Tarantool-patches] [PATCH v4 3/4] http: enrich httpc_request with curl error message buffer Ilya Kosarev 2019-10-28 23:41 ` Alexander Turenko 2019-11-07 12:07 ` Ilya Kosarev 2019-10-28 17:11 ` [Tarantool-patches] [PATCH v4 4/4] http: add CURLE_BAD_CONTENT_ENCODING case for curl_request code Ilya Kosarev 2019-10-28 23:42 ` Alexander Turenko 2019-10-28 23:43 ` [Tarantool-patches] [PATCH v4 0/4] http: add CURLOPT_ACCEPT_ENCODING option and following improvements Alexander Turenko 2019-11-07 12:07 ` Ilya Kosarev
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20191028234216.akbicvptcgov2j3x@tkn_work_nb \ --to=alexander.turenko@tarantool.org \ --cc=i.kosarev@tarantool.org \ --cc=tarantool-patches@dev.tarantool.org \ --cc=tarantool-patches@freelists.org \ --subject='Re: [Tarantool-patches] [PATCH v4 1/4] http: add CURLOPT_ACCEPT_ENCODING option' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox