Tony Foerster has posted comments on this change. ( http://gerrit.cloudera.org:8080/11030 )
Change subject: Scala code formatting with Scalafmt ...................................................................... Patch Set 3: (7 comments) http://gerrit.cloudera.org:8080/#/c/11030/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11030/2//COMMIT_MSG@9 PS2, Line 9: Scalafmt is added using the Gradle Scalafmt plugin: > Have you considered using the spotless plugin? I saw that Spotless would do scalafmt, but also does a lot more, so I went with the simpler solution. If we want to add Java formatting as well Spotless might be the way to go, I don't have any experience with it. I'd feel better about doing the commits for scala/java style separately. If spotless was added in the future it should be trivial to use it for scalafmt. http://gerrit.cloudera.org:8080/#/c/11030/2/java/.scalafmt.conf File java/.scalafmt.conf: http://gerrit.cloudera.org:8080/#/c/11030/2/java/.scalafmt.conf@1 PS2, Line 1: rewrite.rules = [SortImports, sortModifiers, prefercurlyfors, AvoidInfix, expandimportselectors] > I like `expandimportselectors` because it results in less change collisions Done http://gerrit.cloudera.org:8080/#/c/11030/2/java/.scalafmt.conf@2 PS2, Line 2: align = none > +1 this setting is pretty aggressive I don't mind the aligned cases (as long as it's done automatically for me), but two on one I'll let it go :) http://gerrit.cloudera.org:8080/#/c/11030/2/java/.scalafmt.conf@9 PS2, Line 9: newlines.alwaysBeforeTopLevelStatements = true > Here is an example of what this setting can do: https://github.com/olafurpg Agreed, the formatting can get a little too vertical. http://gerrit.cloudera.org:8080/#/c/11030/2/java/gradle/quality.gradle File java/gradle/quality.gradle: http://gerrit.cloudera.org:8080/#/c/11030/2/java/gradle/quality.gradle@25 PS2, Line 25: apply plugin: "scalafmt" > nit: newline Done http://gerrit.cloudera.org:8080/#/c/11030/2/java/kudu-backup/out/test/resources/log4j.properties File java/kudu-backup/out/test/resources/log4j.properties: http://gerrit.cloudera.org:8080/#/c/11030/2/java/kudu-backup/out/test/resources/log4j.properties@1 PS2, Line 1: > OK I've done this many times now. I added the 'out' dirs to the .gitignore, let me know if you'd rather it be its own commit. http://gerrit.cloudera.org:8080/#/c/11030/2/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/package.scala File java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/package.scala: http://gerrit.cloudera.org:8080/#/c/11030/2/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/package.scala@24 PS2, Line 24: * Adds a method, `kudu`, to DataFrameReader that allows you to read Kudu tables using > Why is this using the Scala style comment? Odd, none of them changed to javadoc. Since they all seem to be scaladoc style in the first place I'll switch to that for now. Or maybe it's just broken. -- To view, visit http://gerrit.cloudera.org:8080/11030 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iac96383d88394084e19712177d05f9fc63de766c Gerrit-Change-Number: 11030 Gerrit-PatchSet: 3 Gerrit-Owner: Tony Foerster <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Tony Foerster <[email protected]> Gerrit-Comment-Date: Tue, 24 Jul 2018 19:25:21 +0000 Gerrit-HasComments: Yes
