Sam Okrent has posted comments on this change. Change subject: Integrate Mustache templates to webserver ......................................................................
Patch Set 10: (6 comments) http://gerrit.cloudera.org:8080/#/c/7448/9/src/kudu/server/webserver.cc File src/kudu/server/webserver.cc: Line 479: void Webserver::RenderMainTemplate(const string& content, stringstream* output) { > How come you decided to keep this template inline instead of putting into t It's inlined so that something displays on the page even if the doc root isn't configured (if it was in a file it wouldn't be rendered). The benefits of having the other templates in separate files include - They can be modified without recompiling - You can use syntax highlighting when editing - Naming them by their url path eliminates the need for some kind of lookup table (would be required if they were inlined) I'm moving the main template to it's own file and instead printing out a simple plain-text error message if the doc root isn't configured. http://gerrit.cloudera.org:8080/#/c/7448/8/src/kudu/server/webserver.h File src/kudu/server/webserver.h: Line 63: const PathHandlerCallback& callback, > reindent the param lists here and below. Done http://gerrit.cloudera.org:8080/#/c/7448/9/src/kudu/server/webserver.h File src/kudu/server/webserver.h: Line 67: const PrerenderedPathHandlerCallback& callback, > re-indent here and above Done http://gerrit.cloudera.org:8080/#/c/7448/9/src/kudu/util/easy_json.cc File src/kudu/util/easy_json.cc: Line 187: push_val.SetArray(); > I think this needs a final 'else { LOG(FATAL) << "Unknown initializer type" Done http://gerrit.cloudera.org:8080/#/c/7448/9/src/kudu/util/easy_json.h File src/kudu/util/easy_json.h: Line 120: // Returns the child object. > This method and below don't return a reference, perhaps just 'Returns the c Woops, copy-paste error Line 161: std::string ToString() const; > I think it'd be better to keep ToString without a param, since that matches Done -- To view, visit http://gerrit.cloudera.org:8080/7448 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8cf8181b2b2904a57c9b24ce73316b92a4f6700f Gerrit-PatchSet: 10 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Sam Okrent <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Sam Okrent <[email protected]> Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: Yes
