Hao Hao 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: (7 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. 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? http://gerrit.cloudera.org:8080/#/c/10572/2/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala@192 PS2, Line 192: def kuduType(dt: DataType) : Type = { Same here. 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@40 PS2, Line 40: * @param dt the Spark SQL type nit: add an empty line. 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. 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. 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? -- 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: Hao Hao <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Comment-Date: Fri, 01 Jun 2018 19:02:01 +0000 Gerrit-HasComments: Yes
