Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/10572 )
Change subject: [java] Move some methods to a SparkUtil class ...................................................................... Patch Set 2: (5 comments) http://gerrit.cloudera.org:8080/#/c/10572/2/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala File java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala: http://gerrit.cloudera.org:8080/#/c/10572/2/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala@24 PS2, Line 24: import org.apache.kudu.ColumnSchema > nit: import order. Done http://gerrit.cloudera.org:8080/#/c/10572/2/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala File java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala: http://gerrit.cloudera.org:8080/#/c/10572/2/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala@187 PS2, Line 187: def createSchema(schema: StructType, keys: Seq[String]): Schema = { > Why not remove this method if it is just calling kuduSchema? I didn't want to break public API. http://gerrit.cloudera.org:8080/#/c/10572/2/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/SparkUtil.scala File java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/SparkUtil.scala: http://gerrit.cloudera.org:8080/#/c/10572/2/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/SparkUtil.scala@100 PS2, Line 100: private def createColumn(field: StructField, isKey: Boolean): ColumnSchema = { > nit: method comment. Done http://gerrit.cloudera.org:8080/#/c/10572/2/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/SparkUtil.scala@102 PS2, Line 102: val col = new ColumnSchema.ColumnSchemaBuilder(field.name, kt).key(isKey).nullable(field.nullable) > nit: exceeds line length limit. Done http://gerrit.cloudera.org:8080/#/c/10572/2/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala File java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala: http://gerrit.cloudera.org:8080/#/c/10572/2/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala@35 PS2, Line 35: import scala.collection.JavaConverters._ > Is this needed? Done -- To view, visit http://gerrit.cloudera.org:8080/10572 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib78ec895b25220668757432de287f6d0e5ed5ccc Gerrit-Change-Number: 10572 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Hao Hao <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Comment-Date: Sun, 03 Jun 2018 21:05:01 +0000 Gerrit-HasComments: Yes
