Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17558 )
Change subject: [client] replace Equals() with operator==() ...................................................................... Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/17558/1/src/kudu/client/client-unittest.cc File src/kudu/client/client-unittest.cc: http://gerrit.cloudera.org:8080/#/c/17558/1/src/kudu/client/client-unittest.cc@328 PS1, Line 328: ASSERT_TRUE(a32 != b32); > What additional purpose does this 'ASSERT_TRUE' statement add considering t As with the Equals() method, it's all about testing corresponding functionality: ASSERT_FALSE() is for testing operator==(), ASSERT_TRUE() is for testing operator!=() http://gerrit.cloudera.org:8080/#/c/17558/1/src/kudu/tools/table_scanner.cc File src/kudu/tools/table_scanner.cc: http://gerrit.cloudera.org:8080/#/c/17558/1/src/kudu/tools/table_scanner.cc@354 PS1, Line 354: d > nit: Capitalize 'd' in destination. I strongly prefer to have all error messages starting with non-capital letters: that's because those error messages tend to chain, and capitalized those chained messages look awful. If interested, you can find more details on this policy from GoLang error messages 'style guide'. -- To view, visit http://gerrit.cloudera.org:8080/17558 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4a95534835c93b1fb7a3ca1f311c530572c50ad2 Gerrit-Change-Number: 17558 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy <[email protected]> Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Tue, 08 Jun 2021 06:52:05 +0000 Gerrit-HasComments: Yes
