Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18945 )
Change subject: KUDU-3393 C++ client support split a tablet to mutil ranges and concurrent scan data ...................................................................... Patch Set 8: (3 comments) http://gerrit.cloudera.org:8080/#/c/18945/8/src/kudu/client/client-test.cc File src/kudu/client/client-test.cc: http://gerrit.cloudera.org:8080/#/c/18945/8/src/kudu/client/client-test.cc@8637 PS8, Line 8637: , int diff_value = 120 There is only one caller, and it's always 120, how about set it as a local const variable? http://gerrit.cloudera.org:8080/#/c/18945/8/src/kudu/client/client-test.cc@8649 PS8, Line 8649: diff_value Could you please add some comments to describe what's the purpose of diff_value? http://gerrit.cloudera.org:8080/#/c/18945/8/src/kudu/client/client-test.cc@8739 PS8, Line 8739: ASSERT_LE(4, token_size_b); Is it definite that 4 < token_size_b (and 4 < token_size_a), if it is, better to use ASSERT_LT than ASSERT_LE. If token_size_a == token_size_b == 4, the test will success, but it's not what you want I guess. -- To view, visit http://gerrit.cloudera.org:8080/18945 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I207f9584cd558d32fcd9e8de7d6c25e517377272 Gerrit-Change-Number: 18945 Gerrit-PatchSet: 8 Gerrit-Owner: KeDeng <[email protected]> Gerrit-Reviewer: KeDeng <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Comment-Date: Sun, 06 Nov 2022 12:56:50 +0000 Gerrit-HasComments: Yes
