Alexey Serbin has posted comments on this change.
Change subject: [KuduPartialRow::Set{Binary,String}()] copy input data
......................................................................
Patch Set 9:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/3868/9/src/kudu/common/partial_row-test.cc
File src/kudu/common/partial_row-test.cc:
Line 47: typedef Status (KuduPartialRow::*BinarySetter)(int, const Slice&);
> OK I understand why this is needed now, based on our Slack conversation. So
Yep, that's the fun part about std::function which I didn't know before I tried
to introduce the suggested std::function change.
Line 47: typedef Status (KuduPartialRow::*BinarySetter)(int, const Slice&);
> hrm, would you mind removing the space between Status and the opening paren
The Status is separated by a space because it's a return type. I don't think
removing the space after that would bring in any clarity.
These typedefs are for static_casts<> which you can find below in the code.
It's not possible to instantiate std::function wrapper for overloaded function
without an explicit cast. The cast is necessary to allow the compiler to deal
with ambiguity in the presence of overloaded functions and instantiate the
appropriate template when compiling the code with std::functor wrapper for the
overloaded functions.
Line 78: case COPY:
> style nit: indent case per https://google.github.io/styleguide/cppguide.htm
Done
Line 98: case COPY:
> same
Done
--
To view, visit http://gerrit.cloudera.org:8080/3868
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I764339a0e3ffbf6abd5372e682c9fa1792bdd52b
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <aser...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <d...@cloudera.com>
Gerrit-Reviewer: Dinesh Bhat <din...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mpe...@apache.org>
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-HasComments: Yes
