Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16826 )
Change subject: Replace boost::iequals with our own implementation ...................................................................... Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/16826/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16826/2//COMMIT_MSG@7 PS2, Line 7: our own implementation > Cool, thank you for the clarification. Does it make sense to reflect this Done http://gerrit.cloudera.org:8080/#/c/16826/2/src/kudu/util/string_case.h File src/kudu/util/string_case.h: http://gerrit.cloudera.org:8080/#/c/16826/2/src/kudu/util/string_case.h@61 PS2, Line 61: iequals > Since it's targeted to work as expected only with ASCII symbols, maybe rena I like the idea of adding the DCHECK but keeping the name the same. The other methods in this class also expect ascii but do not add that to the name. In the future we can enhance the impl if needed. http://gerrit.cloudera.org:8080/#/c/16826/2/src/kudu/util/string_case.cc File src/kudu/util/string_case.cc: http://gerrit.cloudera.org:8080/#/c/16826/2/src/kudu/util/string_case.cc@86 PS2, Line 86: bool iequals(const string& a, const string& b) { > Since it's targeted to work only with ASCII characters, does it make sense good idea. I added these checks to the other methods as well. -- To view, visit http://gerrit.cloudera.org:8080/16826 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6d2fdd30b739cee53e1551f784dbebfbe76e9233 Gerrit-Change-Number: 16826 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy <[email protected]> Gerrit-Comment-Date: Tue, 08 Dec 2020 03:46:41 +0000 Gerrit-HasComments: Yes
