You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Mike Percy (Code Review)" <ge...@cloudera.org> on 2017/12/01 09:20:00 UTC
[kudu-CR] [client] expose non-voter replicas for kudu CLI tool
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/8586 )
Change subject: [client] expose non-voter replicas for kudu CLI tool
......................................................................
Patch Set 4:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/8586/3/src/kudu/client/client.cc
File src/kudu/client/client.cc:
http://gerrit.cloudera.org:8080/#/c/8586/3/src/kudu/client/client.cc@a340
PS3, Line 340:
> They are not needed in C++ default constructors. Also, it's less symbols t
When you omit the parentheses, it makes me feel like you are doing it for a reason, because we don't normally do that in the Kudu code base... admittedly probably because most of us came to C++ from Java. However can be a semantic difference of leaving off the parenthesis when calling a constructor in C++, depending on context: https://stackoverflow.com/questions/620137/do-the-parentheses-after-the-type-name-make-a-difference-with-new
So while I'm sure there is no semantic difference in this context, it seems like we should only do this when we truly expect and want no initialization on a POD object for efficiency reasons, don't you think?
On the other hand, I suppose we do it quite often when creating objects on the stack, like Foo f; so maybe I am overreacting here?
Please let me know what you think.
http://gerrit.cloudera.org:8080/#/c/8586/4/src/kudu/client/client.cc
File src/kudu/client/client.cc:
http://gerrit.cloudera.org:8080/#/c/8586/4/src/kudu/client/client.cc@550
PS4, Line 550: e
nit: End the comment with a period per the style guide.
--
To view, visit http://gerrit.cloudera.org:8080/8586
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I19317fdf5a2d5c8bb5f37b27bb83067a4df4ea20
Gerrit-Change-Number: 8586
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Fri, 01 Dec 2017 09:20:00 +0000
Gerrit-HasComments: Yes