impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Matthew Jacobs (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] Bump Kudu version to 1c70e5d
Date Tue, 29 Aug 2017 17:55:47 GMT
Matthew Jacobs has posted comments on this change.

Change subject: Bump Kudu version to 1c70e5d
......................................................................


Patch Set 1:

(1 comment)

does the code not compile without the header changes? It's best to separate code changes from
toolchain version bumps when possible because it makes porting to other branches harder. It's
OK if it's not possible.

http://gerrit.cloudera.org:8080/#/c/7872/1/be/src/exprs/kudu-partition-expr.h
File be/src/exprs/kudu-partition-expr.h:

PS1, Line 21: #include <kudu/client/client.h>
            : #include <kudu/common/partial_row.h>
can you try to just forward declare instead of including here (then including in the .cc)?
for both kudu::client::KuduPartitioner and kudu::KuduPartialRow? I think that a unique_ptr
doesn't need the full class definition.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd041b783a9b7764f4b251291e0be5ed000485ce
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall <tmarshall@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message