Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/14831 )
Change subject: KUDU-1938 Add VARCAHR to tooling and helpers pt 7 ...................................................................... Patch Set 1: > Patch Set 1: > > Could you add appropriate test coverage to exercise the new bits? The change in key_util.cc wasn't necessary after all as it checks physical types so the STRING case is unnecessary too. Could still add test and remove the STRING case. The methods in row.h are only convenient methods, most of them are not used at all, but it might make sense to still add them. The tool actions should be tested, but unfortunately they don't have tests at all (they would've likely caught the missing VARCHAR handling), but adding the tests would be a scope creep I believe. How do you feel about filing a JIRA to test the tools? -- To view, visit http://gerrit.cloudera.org:8080/14831 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I84257c71528dafdf9c01c0d3cdda44a626ed7f67 Gerrit-Change-Number: 14831 Gerrit-PatchSet: 1 Gerrit-Owner: Attila Bukor <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 04 Dec 2019 22:33:34 +0000 Gerrit-HasComments: No
