You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Alexey Serbin (Code Review)" <ge...@cloudera.org> on 2016/09/22 02:20:42 UTC
[kudu-CR] [c++compilation] fixed 'unused' warnings
Alexey Serbin has uploaded a new change for review.
http://gerrit.cloudera.org:8080/4503
Change subject: [c++compilation] fixed 'unused' warnings
......................................................................
[c++compilation] fixed 'unused' warnings
Use DCHECK() instead of DCHECK_NOTNULL() as recommended by
glog/logging.h to avoid compilation warnings in release configuration.
An example of previously emitted warning:
src/kudu/common/wire_protocol.cc:599:18:
warning:
expression result unused [-Wunused-value]
DCHECK_NOTNULL(dst_schema);
^~~~~~~~~~
thirdparty/installed-deps/include/glog/logging.h:1044:30:
note:
expanded from macro 'DCHECK_NOTNULL'
Also, moved schema partitioning compatibility function under the
ifdef to fix the unused function warning.
Change-Id: If0a59ef51e5c5ea8be89109a48a57dc5abfde646
---
M src/kudu/client/meta_cache.h
M src/kudu/codegen/row_projector.cc
M src/kudu/common/wire_protocol.cc
M src/kudu/consensus/consensus.cc
M src/kudu/consensus/raft_consensus.cc
M src/kudu/rpc/rpc.cc
6 files changed, 62 insertions(+), 64 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/03/4503/1
--
To view, visit http://gerrit.cloudera.org:8080/4503
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: If0a59ef51e5c5ea8be89109a48a57dc5abfde646
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
[kudu-CR] [c++compilation] fixed 'unused' warnings
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: [c++compilation] fixed 'unused' warnings
......................................................................
Patch Set 1: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4503/1/src/kudu/codegen/row_projector.cc
File src/kudu/codegen/row_projector.cc:
PS1, Line 429: auto compat_check = [](const Schema& base1, const Schema& proj1,
: const Schema& base2, const Schema& proj2) {
You find the use of a lambda to be cleaner than just surrounding the function definition with #ifndef NDEBUG? Okay...
--
To view, visit http://gerrit.cloudera.org:8080/4503
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If0a59ef51e5c5ea8be89109a48a57dc5abfde646
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] [c++compilation] fixed 'unused' warnings
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: [c++compilation] fixed 'unused' warnings
......................................................................
Patch Set 1:
(1 comment)
Oops, it seems I pressed 'Submit' instead of reply. Anyway, I'm definitely open to change it.
http://gerrit.cloudera.org:8080/#/c/4503/1/src/kudu/codegen/row_projector.cc
File src/kudu/codegen/row_projector.cc:
PS1, Line 429: auto compat_check = [](const Schema& base1, const Schema& proj1,
: const Schema& base2, const Schema& proj2) {
> You find the use of a lambda to be cleaner than just surrounding the functi
Well, for some reason I didn't feel comfortable putting it under ifdef.
Besides, I didn't see any other places this method is used and there aren't any other analogous methods around.
Probably, there might be some performance penalties. If you have additional reasons to move it under ifdef, I will definitely reconsider this.
--
To view, visit http://gerrit.cloudera.org:8080/4503
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If0a59ef51e5c5ea8be89109a48a57dc5abfde646
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] [c++compilation] fixed 'unused' warnings
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged.
Change subject: [c++compilation] fixed 'unused' warnings
......................................................................
[c++compilation] fixed 'unused' warnings
Use DCHECK() instead of DCHECK_NOTNULL() as recommended by
glog/logging.h to avoid compilation warnings in release configuration.
An example of previously emitted warning:
src/kudu/common/wire_protocol.cc:599:18:
warning:
expression result unused [-Wunused-value]
DCHECK_NOTNULL(dst_schema);
^~~~~~~~~~
thirdparty/installed-deps/include/glog/logging.h:1044:30:
note:
expanded from macro 'DCHECK_NOTNULL'
Also, moved schema partitioning compatibility function under the
ifdef to fix the unused function warning.
Change-Id: If0a59ef51e5c5ea8be89109a48a57dc5abfde646
Reviewed-on: http://gerrit.cloudera.org:8080/4503
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo <ad...@cloudera.com>
---
M src/kudu/client/meta_cache.h
M src/kudu/codegen/row_projector.cc
M src/kudu/common/wire_protocol.cc
M src/kudu/consensus/consensus.cc
M src/kudu/consensus/raft_consensus.cc
M src/kudu/rpc/rpc.cc
6 files changed, 62 insertions(+), 64 deletions(-)
Approvals:
Adar Dembo: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/4503
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: If0a59ef51e5c5ea8be89109a48a57dc5abfde646
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [c++compilation] fixed 'unused' warnings
Posted by "Dinesh Bhat (Code Review)" <ge...@cloudera.org>.
Dinesh Bhat has posted comments on this change.
Change subject: [c++compilation] fixed 'unused' warnings
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4503/1/src/kudu/codegen/row_projector.cc
File src/kudu/codegen/row_projector.cc:
PS1, Line 408: ProjectionsCompatible
Sorry looked at this little late; Looks like this is a non-existant routine name now, I will clean this up in my patch if it's okay.
--
To view, visit http://gerrit.cloudera.org:8080/4503
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If0a59ef51e5c5ea8be89109a48a57dc5abfde646
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes