Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/10332 )
Change subject: KUDU-2035 Enable HTTP compression for all webserver's paths ...................................................................... Patch Set 6: (4 comments) A few nits, but overall it's good. Also, please address the automatically generated feedback from the Tidy Bot. http://gerrit.cloudera.org:8080/#/c/10332/3/src/kudu/server/webserver-test.cc File src/kudu/server/webserver-test.cc: http://gerrit.cloudera.org:8080/#/c/10332/3/src/kudu/server/webserver-test.cc@188 PS3, Line 188: Should > I followed the comment style from the 'TestIndexPage' above. Do I need to c Nope, I think in that case it's better just to keep it consistent. http://gerrit.cloudera.org:8080/#/c/10332/3/src/kudu/server/webserver-test.cc@189 PS3, Line 189: ASSERT_STR_CONTAINS(buf_.ToString(), "Kudu"); > Done. 'Host' is in request headers but not in response headers tho? Woops, indeed. Sorry for messing it up. http://gerrit.cloudera.org:8080/#/c/10332/6/src/kudu/util/curl_util.h File src/kudu/util/curl_util.h: http://gerrit.cloudera.org:8080/#/c/10332/6/src/kudu/util/curl_util.h@45 PS6, Line 45: i.e. nit: I think 'e.g.' is more appropriate here because that's just an example, right? http://gerrit.cloudera.org:8080/#/c/10332/3/src/kudu/util/curl_util.cc File src/kudu/util/curl_util.cc: http://gerrit.cloudera.org:8080/#/c/10332/3/src/kudu/util/curl_util.cc@90 PS3, Line 90: to cl > Which one? I think it's ok for the 'chunk' to be null? I meant curl_slist_append() returns nullptr if anything went wrong. I think it's worth handling that situation here as well. -- To view, visit http://gerrit.cloudera.org:8080/10332 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7c6db24b0fd2cbcca8a554460d310bd39ee5c071 Gerrit-Change-Number: 10332 Gerrit-PatchSet: 6 Gerrit-Owner: Fengling Wang <fw...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Fengling Wang <fw...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Will Berkeley <wdberke...@gmail.com> Gerrit-Comment-Date: Mon, 14 May 2018 18:01:38 +0000 Gerrit-HasComments: Yes
