Alexey Serbin has posted comments on this change.

Change subject: C++ client: deprecating KuduPartialRow::SetString()

Patch Set 3:

Commit Message:

Line 21: An alternative approach might be to change behavior of
> That would break ABI compatibility and it doesn't seem to be really necessa
OK, for the other approach a created a separate changeset:
File src/kudu/common/

Line 289: Status KuduPartialRow::SetBinaryNoCopy(const Slice& col_name, const 
Slice& val) {
> style note: If you change the order in the header file please also change t
File src/kudu/common/partial_row.h:

Line 98:   // Set the string/binary value but does not copy the value. The slice
> style nit: It makes more sense to me to keep all the string methods togethe
Good point!  However, the point here is to emphasize the groups of different 

> style nit: Line continuations should be 4 spaces, not 2. See https://google

Line 110:       "use SetStringNoCopy() instead; also consider using 
> style: Another line continuation, add an additional 4 spaces more than the 

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I348b0b9437b8d7928e3b607a0e0610d8d0c58f7c
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <>
Gerrit-Reviewer: Adar Dembo <>
Gerrit-Reviewer: Alexey Serbin <>
Gerrit-Reviewer: Dan Burkert <>
Gerrit-Reviewer: Dinesh Bhat <>
Gerrit-Reviewer: Jean-Daniel Cryans <>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <>
Gerrit-Reviewer: Todd Lipcon <>
Gerrit-HasComments: Yes

Reply via email to