Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14219 )
Change subject: KUDU-1561 Implemented operator->() in KuduScanBatch::const_iterator ...................................................................... Patch Set 1: (5 comments) http://gerrit.cloudera.org:8080/#/c/14219/1/src/kudu/client/client-test.cc File src/kudu/client/client-test.cc: http://gerrit.cloudera.org:8080/#/c/14219/1/src/kudu/client/client-test.cc@5622 PS1, Line 5622: ++it Is it important to work with the second (not first) element in the batch? If so, please add a comment explaining why. http://gerrit.cloudera.org:8080/#/c/14219/1/src/kudu/client/client-test.cc@5626 PS1, Line 5626: CHECK_EQ Would ASSERT_EQ() suffice here or it's really important to crash if not? http://gerrit.cloudera.org:8080/#/c/14219/1/src/kudu/client/scan_batch.h File src/kudu/client/scan_batch.h: http://gerrit.cloudera.org:8080/#/c/14219/1/src/kudu/client/scan_batch.h@165 PS1, Line 165: /// Overloaded operator -> to support pointer trait Please describe what this method returns using @return doxygen notation (see other methods in this file for examples). http://gerrit.cloudera.org:8080/#/c/14219/1/src/kudu/client/scan_batch.h@166 PS1, Line 166: (KUDU-1561) nit: I think it's better not to put those details in the outer-facing documentation since from this text an auto-generated doc is published at https://kudu.apache.org/cpp-client-api/index.html http://gerrit.cloudera.org:8080/#/c/14219/1/src/kudu/client/scan_batch.h@343 PS1, Line 343: /// Since iterator does not keep current KuduScanBatch::RowPtr, : /// we cannot return pointer to it. : /// Instead we return KuduScanBatch::RowPtr, : /// which implements pointer operator -> nit: move the details/reasoning under a @note section. -- To view, visit http://gerrit.cloudera.org:8080/14219 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib5ddd99073d6a93337c184bee8b930cabeeda145 Gerrit-Change-Number: 14219 Gerrit-PatchSet: 1 Gerrit-Owner: Volodymyr Verovkin <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Thu, 12 Sep 2019 16:36:21 +0000 Gerrit-HasComments: Yes
