[kudu-CR] KUDU-1492: Show column encodings/compression on table page in master Test results are in JIRA: https://issues.apache.org/jira/browse/KUDU-1492
Alexey Serbin has posted comments on this change. Change subject: KUDU-1492: Show column encodings/compression on table page in master Test results are in JIRA: https://issues.apache.org/jira/browse/KUDU-1492 .. Patch Set 1: Code-Review+1 (2 comments) LGTM, just a couple questions. http://gerrit.cloudera.org:8080/#/c/3667/1/src/kudu/server/webui_util.cc File src/kudu/server/webui_util.cc: Line 35:std::stringstream* output) { This is not part of this change, but just an additional comment. Does it make sense to change type of output into more narrower type ostringstream since the method is supposed only to output some data in there? Line 56: *output << Substitute("$0$1$2$3" I, as a reader, would appreciate some text example of the output in the comment. Would it make sense? -- To view, visit http://gerrit.cloudera.org:8080/3667 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I990a8d790ab71a05be04f0b7468b5da0894478e8 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: Yes
[kudu-CR] KUDU-1492: Show column encodings/compression on table page in master Test results are in JIRA: https://issues.apache.org/jira/browse/KUDU-1492
Mike Percy has posted comments on this change. Change subject: KUDU-1492: Show column encodings/compression on table page in master Test results are in JIRA: https://issues.apache.org/jira/browse/KUDU-1492 .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/3667/1//COMMIT_MSG Commit Message: Line 7: KUDU-1492: Show column encodings/compression on table page in master > leave a blank line between the commit title and the description. This is a good summary of best practices for this: http://chris.beams.io/posts/git-commit/#seven-rules http://gerrit.cloudera.org:8080/#/c/3667/1/src/kudu/server/webui_util.cc File src/kudu/server/webui_util.cc: Line 39: << "Encoding/Compression" How about we use two different columns for each of encoding and compression? -- To view, visit http://gerrit.cloudera.org:8080/3667 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I990a8d790ab71a05be04f0b7468b5da0894478e8 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: Yes
[kudu-CR] KUDU-1492: Show column encodings/compression on table page in master Test results are in JIRA: https://issues.apache.org/jira/browse/KUDU-1492
David Ribeiro Alves has posted comments on this change. Change subject: KUDU-1492: Show column encodings/compression on table page in master Test results are in JIRA: https://issues.apache.org/jira/browse/KUDU-1492 .. Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/3667/1//COMMIT_MSG Commit Message: Line 7: KUDU-1492: Show column encodings/compression on table page in master leave a blank line between the commit title and the description. Line 8: Test results are in JIRA: https://issues.apache.org/jira/browse/KUDU-1492 Usually our commit messages are a little more informative (see other commits) but this is simple enough that I understand if you want to leave it like that. Also max 80 cols on commit messages. Not super strict on that but it would be nice. http://gerrit.cloudera.org:8080/#/c/3667/1/src/kudu/server/webui_util.cc File src/kudu/server/webui_util.cc: PS1, Line 40: spaces instead of tabs. check out google style guide. PS1, Line 55: same -- To view, visit http://gerrit.cloudera.org:8080/3667 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I990a8d790ab71a05be04f0b7468b5da0894478e8 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: Yes
[kudu-CR] KUDU-1492: Show column encodings/compression on table page in master Test results are in JIRA: https://issues.apache.org/jira/browse/KUDU-1492
Kudu Jenkins has posted comments on this change. Change subject: KUDU-1492: Show column encodings/compression on table page in master Test results are in JIRA: https://issues.apache.org/jira/browse/KUDU-1492 .. Patch Set 1: Build Started http://104.196.14.100/job/kudu-gerrit/2520/ -- To view, visit http://gerrit.cloudera.org:8080/3667 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I990a8d790ab71a05be04f0b7468b5da0894478e8 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] KUDU-1492: Show column encodings/compression on table page in master Test results are in JIRA: https://issues.apache.org/jira/browse/KUDU-1492
Hello Jean-Daniel Cryans, Mike Percy, Adar Dembo, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/3667 to review the following change. Change subject: KUDU-1492: Show column encodings/compression on table page in master Test results are in JIRA: https://issues.apache.org/jira/browse/KUDU-1492 .. KUDU-1492: Show column encodings/compression on table page in master Test results are in JIRA: https://issues.apache.org/jira/browse/KUDU-1492 Change-Id: I990a8d790ab71a05be04f0b7468b5da0894478e8 --- M src/kudu/server/webui_util.cc 1 file changed, 8 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/67/3667/1 -- To view, visit http://gerrit.cloudera.org:8080/3667 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I990a8d790ab71a05be04f0b7468b5da0894478e8 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Mike Percy