You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Todd Lipcon (Code Review)" <ge...@cloudera.org> on 2020/08/12 21:27:52 UTC

[kudu-CR] KUDU-2844 (2/3): move RowBlock memory into a new RowBlockMemory struct

Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/15801 )

Change subject: KUDU-2844 (2/3): move RowBlock memory into a new RowBlockMemory struct
......................................................................


Patch Set 3:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/15801/3/src/kudu/cfile/cfile-test.cc
File src/kudu/cfile/cfile-test.cc:

http://gerrit.cloudera.org:8080/#/c/15801/3/src/kudu/cfile/cfile-test.cc@639
PS3, Line 639:   ASSERT_EQ(5000u, iter->GetCurrentOrdinal());
> warning: integer literal has suffix 'u', which is not uppercase [readabilit
Done


http://gerrit.cloudera.org:8080/#/c/15801/3/src/kudu/cfile/cfile-test.cc@706
PS3, Line 706:   EXPECT_EQ(0u, iter->GetCurrentOrdinal());
> warning: integer literal has suffix 'u', which is not uppercase [readabilit
Done


http://gerrit.cloudera.org:8080/#/c/15801/3/src/kudu/cfile/cfile-test.cc@712
PS3, Line 712:   ASSERT_EQ(0u, iter->GetCurrentOrdinal());
> warning: integer literal has suffix 'u', which is not uppercase [readabilit
Done


http://gerrit.cloudera.org:8080/#/c/15801/3/src/kudu/tablet/memrowset-test.cc
File src/kudu/tablet/memrowset-test.cc:

http://gerrit.cloudera.org:8080/#/c/15801/3/src/kudu/tablet/memrowset-test.cc@216
PS3, Line 216:       mem.Reset();
> This wasn't necessary, was it? It's just good practice?
yep


http://gerrit.cloudera.org:8080/#/c/15801/3/src/kudu/tablet/tablet-decoder-eval-test.cc
File src/kudu/tablet/tablet-decoder-eval-test.cc:

http://gerrit.cloudera.org:8080/#/c/15801/3/src/kudu/tablet/tablet-decoder-eval-test.cc@267
PS3, Line 267:     string column_str_a, column_str_b;
> warning: multiple declarations in a single statement reduces readability [r
Done


http://gerrit.cloudera.org:8080/#/c/15801/3/src/kudu/tablet/tablet-test-util.h
File src/kudu/tablet/tablet-test-util.h:

http://gerrit.cloudera.org:8080/#/c/15801/3/src/kudu/tablet/tablet-test-util.h@200
PS3, Line 200: static inline Status SilentIterateToStringList(RowwiseIterator* iter,
> warning: unused function 'SilentIterateToStringList' [clang-diagnostic-unus
this function is actually used by other files that include it



-- 
To view, visit http://gerrit.cloudera.org:8080/15801
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I17a21f33f44988795ffe064b3ba41055e1a19e90
Gerrit-Change-Number: 15801
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Wed, 12 Aug 2020 21:27:52 +0000
Gerrit-HasComments: Yes