Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12061 )
Change subject: [Java] Add a Schema and Data Generator ...................................................................... Patch Set 1: (7 comments) http://gerrit.cloudera.org:8080/#/c/12061/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/12061/1//COMMIT_MSG@9 PS1, Line 9: This patch adds a schema and data generator utility : class Nit: This patch adds schema and data generate utility classes http://gerrit.cloudera.org:8080/#/c/12061/1//COMMIT_MSG@11 PS1, Line 11: usefull useful http://gerrit.cloudera.org:8080/#/c/12061/1/java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java File java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java: PS1: License header. http://gerrit.cloudera.org:8080/#/c/12061/1/java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java@58 PS1, Line 58: public void randomizeRow(PartialRow row) { Seems redundant to have both randomizeRow and randomRow. The TestKuduBackup case shows a need for randomizeRow; can we omit randomRow? If not, could you at least rename one? The two names are quite similar. http://gerrit.cloudera.org:8080/#/c/12061/1/java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java@97 PS1, Line 97: i++; It's easy to miss this; perhaps convert into a for loop on i (capped at columns.size())? http://gerrit.cloudera.org:8080/#/c/12061/1/java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java@129 PS1, Line 129: public static class DataGeneratorBuilder { Since the only way to create a DataGenerator is through here, maybe doc this a bit? http://gerrit.cloudera.org:8080/#/c/12061/1/java/kudu-client/src/main/java/org/apache/kudu/util/SchemaGenerator.java File java/kudu-client/src/main/java/org/apache/kudu/util/SchemaGenerator.java: PS1: License header. -- To view, visit http://gerrit.cloudera.org:8080/12061 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I750d2d346c3eeb7075b21c3fec0fd25236da4f56 Gerrit-Change-Number: 12061 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 11 Dec 2018 06:54:53 +0000 Gerrit-HasComments: Yes
