Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/11612 )
Change subject: Cleanup in rowset SVG layout dumping ...................................................................... Patch Set 4: (4 comments) http://gerrit.cloudera.org:8080/#/c/11612/3/src/kudu/tablet/svg_dump.cc File src/kudu/tablet/svg_dump.cc: http://gerrit.cloudera.org:8080/#/c/11612/3/src/kudu/tablet/svg_dump.cc@a73 PS3, Line 73: : > nit: why to drop those comments? Are those wrong or outdated? I just felt they didn't say any more than the code itself. http://gerrit.cloudera.org:8080/#/c/11612/3/src/kudu/tablet/svg_dump.cc@a70 PS3, Line 70: : : : : > nit: let's keep these Done http://gerrit.cloudera.org:8080/#/c/11612/3/src/kudu/tablet/svg_dump.cc@a89 PS3, Line 89: : > nit: and these Done http://gerrit.cloudera.org:8080/#/c/11612/3/src/kudu/tablet/svg_dump.cc@147 PS3, Line 147: } : : void PrintXMLHeader(ostream* out) { : CHECK(out); : CHECK(out->go > nit: maybe, convert this into a multi-line snippet? What do you mean by that? If you extend a raw literal over multiple lines, you cannot use indentation in later lines because it will be interpreted as part of the literal. That makes splitting up the literal much superior. Do you mean replacing the sequence of literals and endl's joined by <<'s with literals including \n's? I don't see how that is preferable (it's not really worse, just not worth changing). -- To view, visit http://gerrit.cloudera.org:8080/11612 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I521fadf4336f9e11ea611b022cd4d8fcb269dfe2 Gerrit-Change-Number: 11612 Gerrit-PatchSet: 4 Gerrit-Owner: Will Berkeley <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Fengling Wang <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Reviewer: Will Berkeley <[email protected]> Gerrit-Comment-Date: Tue, 09 Oct 2018 05:43:07 +0000 Gerrit-HasComments: Yes
