Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19633 )
Change subject: [Python] Refactor tests to use assertRaisesRegex ...................................................................... Patch Set 6: Code-Review+1 (3 comments) Overall LGTM, just a few nits. http://gerrit.cloudera.org:8080/#/c/19633/6//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19633/6//COMMIT_MSG@18 PS6, Line 18: renamed to assertRaisesRegex : (without the trailing 'p'). [2] To be able to use assertRaisesRegex nit for here and above: I'm not sure I understand this notation -- what does reference [2] belongs to, to the sentence before the period of after? If the former, maybe put period after the reference? http://gerrit.cloudera.org:8080/#/c/19633/6//COMMIT_MSG@21 PS6, Line 21: , to handle this issue nit: drop this part? http://gerrit.cloudera.org:8080/#/c/19633/6/python/kudu/compat.py File python/kudu/compat.py: http://gerrit.cloudera.org:8080/#/c/19633/6/python/kudu/compat.py@105 PS6, Line 105: class CompatUnitTest(unittest.TestCase): : pass What about Python 3.1? Does it work as expected in attempt to call unittest.TestCase.assertRaisesRegex()? If Python 3.1 isn't supported here, maybe add an explicit comment about that? -- To view, visit http://gerrit.cloudera.org:8080/19633 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I41e2d69996ee0ed0f0418ae184d95239f2739efb Gerrit-Change-Number: 19633 Gerrit-PatchSet: 6 Gerrit-Owner: Marton Greber <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber <[email protected]> Gerrit-Reviewer: Zoltan Chovan <[email protected]> Gerrit-Comment-Date: Tue, 04 Apr 2023 04:55:26 +0000 Gerrit-HasComments: Yes
