[kudu-CR] [c++compilation] fixed 'unused' warnings

2016-09-22 Thread Dinesh Bhat (Code Review)
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 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes


[kudu-CR] [c++compilation] fixed 'unused' warnings

2016-09-21 Thread Alexey Serbin (Code Review)
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 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes


[kudu-CR] [c++compilation] fixed 'unused' warnings

2016-09-21 Thread Adar Dembo (Code Review)
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 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes


[kudu-CR] [c++compilation] fixed 'unused' warnings

2016-09-21 Thread Alexey Serbin (Code Review)
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