Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20597 )

Change subject: Minor refactors on kudu-tool-test
......................................................................


Patch Set 3: Code-Review+1

(8 comments)

http://gerrit.cloudera.org:8080/#/c/20597/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/20597/3//COMMIT_MSG@7
PS3, Line 7: refactors
refactoring ?


http://gerrit.cloudera.org:8080/#/c/20597/3//COMMIT_MSG@9
PS3, Line 9: This patch makes minor refactors on kudu-tool-test.cc,
           : includes
How about:

This patch refactors the code in kudu-tool-test.cc by:


http://gerrit.cloudera.org:8080/#/c/20597/3//COMMIT_MSG@12
PS3, Line 12: Passes
Passing


http://gerrit.cloudera.org:8080/#/c/20597/3//COMMIT_MSG@14
PS3, Line 14: parameters
flags


http://gerrit.cloudera.org:8080/#/c/20597/3//COMMIT_MSG@15
PS3, Line 15: parenthesis
Those are curly brackets, not parentheses.  Parentheses are round brackets.  
See https://en.wikipedia.org/wiki/Parenthesis_(rhetoric) for more details.

I'd rather summarized that update as:

- Introducing appropriate local scoping in the code for better readability


http://gerrit.cloudera.org:8080/#/c/20597/3//COMMIT_MSG@15
PS3, Line 15: with
into


http://gerrit.cloudera.org:8080/#/c/20597/3//COMMIT_MSG@15
PS3, Line 15: Wrap
Wrapping


http://gerrit.cloudera.org:8080/#/c/20597/3/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/20597/3/src/kudu/tools/kudu-tool-test.cc@274
PS3, Line 274:  Now only --encrypt_data_at_rest and
             :     // --enable_multi_tenancy are used in the tests.
I was not able to comprehend this even after re-reading three times.  I thought 
tests add any flags they need, no?

Maybe, you could rephrase this sentence a bit?

Thanks!



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5cba40850d2d2ef884f45e252b8117ae12b8f9d9
Gerrit-Change-Number: 20597
Gerrit-PatchSet: 3
Gerrit-Owner: Yingchun Lai <laiyingc...@apache.org>
Gerrit-Reviewer: Alexey Serbin <ale...@apache.org>
Gerrit-Reviewer: Attila Bukor <abu...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <1450306...@qq.com>
Gerrit-Reviewer: Yingchun Lai <laiyingc...@apache.org>
Gerrit-Comment-Date: Tue, 24 Oct 2023 22:57:42 +0000
Gerrit-HasComments: Yes

Reply via email to