Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/11693 )
Change subject: Fix thrift operator< implementations ...................................................................... Patch Set 4: (4 comments) http://gerrit.cloudera.org:8080/#/c/11693/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11693/4//COMMIT_MSG@7 PS4, Line 7: Fix thrift operator< implementations > Not related to this commit per se, but how do you feel about the fact that Obviously it's not ideal that thrift works this way, but our hands are tied unless we want to change thrift or sentry. http://gerrit.cloudera.org:8080/#/c/11693/4/src/kudu/sentry/thrift_operators-test.cc File src/kudu/sentry/thrift_operators-test.cc: http://gerrit.cloudera.org:8080/#/c/11693/4/src/kudu/sentry/thrift_operators-test.cc@51 PS4, Line 51: ::sentry::TSentryRole role_a; > I left similar feedback on Hao's patches, but could we add "using" statemen done in https://gerrit.cloudera.org/c/11712/ http://gerrit.cloudera.org:8080/#/c/11693/4/src/kudu/sentry/thrift_operators-test.cc@72 PS4, Line 72: // TSentryPrivilege::operator< > As written, it seems that this test and the test for TSentryAuthorizable hi Done, see https://gerrit.cloudera.org/c/11713/. I added the constants as well as an additional case for objects which only differ by table. http://gerrit.cloudera.org:8080/#/c/11693/4/src/kudu/sentry/thrift_operators-test.cc@86 PS4, Line 86: > Nit: got an extra empty line here. Done -- To view, visit http://gerrit.cloudera.org:8080/11693 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1aacf602c603b05433c357a4a236ba0b9e521392 Gerrit-Change-Number: 11693 Gerrit-PatchSet: 4 Gerrit-Owner: Dan Burkert <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Hao Hao <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Wed, 17 Oct 2018 19:54:53 +0000 Gerrit-HasComments: Yes
