[GitHub] spark pull request #23148: [SPARK-26177] Automated formatting for Scala code
Github user asfgit closed the pull request at: https://github.com/apache/spark/pull/23148 --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #23148: [SPARK-26177] Automated formatting for Scala code
Github user koeninger commented on a diff in the pull request:
https://github.com/apache/spark/pull/23148#discussion_r236858392
--- Diff: dev/.scalafmt.conf ---
@@ -0,0 +1,24 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+#http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+align = none
+align.openParenDefnSite = false
+align.openParenCallSite = false
+align.tokens = []
+docstrings = JavaDoc
+maxColumn = 98
--- End diff --
For now, this won't complain about any code, it just gives a way for
contributors to format it.
If we did turn on test as part of the build process, it would complain
about any code that was different after formatting. That could include code in
that 99-101 column range, depending on whether it hit that corner case. E.g.
```
// this is 101 columns and will fail scalastyle, but will pass with
scalafmt set to 99
if
(caseInsensitiveParams.contains(s"kafka.${ConsumerConfig.VALUE_DESERIALIZER_CLASS_CONFIG}"))
{
```
My thinking is that being slightly more strict than existing scalastyle is
better than having automated formatting that won't pass other existing
automated checks. Trying to find and upstream fixes to off-by-one errors in
scalastyle is another option, but that will take a while.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #23148: [SPARK-26177] Automated formatting for Scala code
Github user dbtsai commented on a diff in the pull request: https://github.com/apache/spark/pull/23148#discussion_r236830497 --- Diff: dev/.scalafmt.conf --- @@ -0,0 +1,24 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +#http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +align = none +align.openParenDefnSite = false +align.openParenCallSite = false +align.tokens = [] +docstrings = JavaDoc +maxColumn = 98 --- End diff -- If we set it as `98`, will this complain for legit code with 100 chars? --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #23148: [SPARK-26177] Automated formatting for Scala code
Github user koeninger commented on a diff in the pull request: https://github.com/apache/spark/pull/23148#discussion_r236824453 --- Diff: dev/.scalafmt.conf --- @@ -0,0 +1,24 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +#http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +align = none +align.openParenDefnSite = false +align.openParenCallSite = false +align.tokens = [] +docstrings = JavaDoc +maxColumn = 98 --- End diff -- I originally had this set to 100, and noticed a corner case where an ending space and curly brace were formatted on columns 100 and 101 respectively, making scalastyle complain. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #23148: [SPARK-26177] Automated formatting for Scala code
Github user dbtsai commented on a diff in the pull request: https://github.com/apache/spark/pull/23148#discussion_r236813956 --- Diff: dev/.scalafmt.conf --- @@ -0,0 +1,24 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +#http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +align = none +align.openParenDefnSite = false +align.openParenCallSite = false +align.tokens = [] +docstrings = JavaDoc +maxColumn = 98 --- End diff -- Are we using 100 for maxColumn? --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #23148: [SPARK-26177] Automated formatting for Scala code
Github user koeninger commented on a diff in the pull request: https://github.com/apache/spark/pull/23148#discussion_r236423672 --- Diff: .scalafmt.conf --- @@ -0,0 +1,24 @@ +# --- End diff -- Sure, moved --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #23148: [SPARK-26177] Automated formatting for Scala code
Github user koeninger commented on a diff in the pull request: https://github.com/apache/spark/pull/23148#discussion_r236423438 --- Diff: pom.xml --- @@ -156,6 +156,10 @@ 3.2.2 2.12.7 2.12 +1.5.1 --- End diff -- I moved the scalafmt version inline. The others I think need to stay as parameters so the shell wrapper can override them, to distinguish between the use cases of build pipeline vs contributor fixing code. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #23148: [SPARK-26177] Automated formatting for Scala code
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/23148#discussion_r236421304 --- Diff: .scalafmt.conf --- @@ -0,0 +1,24 @@ +# --- End diff -- Can this file live in dev/? I tried to stuff as many things in there as I could to keep the top-level dir a little less busy. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #23148: [SPARK-26177] Automated formatting for Scala code
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/23148#discussion_r236421583 --- Diff: pom.xml --- @@ -156,6 +156,10 @@ 3.2.2 2.12.7 2.12 +1.5.1 --- End diff -- If these are all just used in one place, in the plugin config, I'd just inline them below to keep it together. If they get reused in more than one place and have to stay synced, then this makes sense. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #23148: [SPARK-26177] Automated formatting for Scala code
GitHub user koeninger opened a pull request: https://github.com/apache/spark/pull/23148 [SPARK-26177] Automated formatting for Scala code ## What changes were proposed in this pull request? Add a maven plugin and wrapper script at ./dev/scalafmt to use scalafmt to format files that differ from git master. Intention is for contributors to be able to use this to automate fixing code style, not to include it in build pipeline yet. If this PR is accepted, I'd make a different PR to update the code style section of https://spark.apache.org/contributing.html to mention the script ## How was this patch tested? Manually tested by modifying a few files and running the script, then checking that scalastyle still passed. You can merge this pull request into a Git repository by running: $ git pull https://github.com/koeninger/spark-1 scalafmt Alternatively you can review and apply these changes as the patch at: https://github.com/apache/spark/pull/23148.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #23148 commit 5df854a340f5e9e2fe7ba60194b4891235420369 Author: cody koeninger Date: 2018-11-21T20:32:57Z WIP on scalafmt config commit 57684272b60ff1213c957541f1db3f9d7aba1543 Author: cody koeninger Date: 2018-11-21T21:22:15Z mvn plugin, e.g. ./build/mvn mvn-scalafmt_2.12:format commit bd7baeca577bf9b519fe028d1e831fb7193e7af9 Author: cody koeninger Date: 2018-11-22T17:07:06Z shell wrapper for mvn scalafmt plugin --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
