Repace Scalastyle with scalafmt #74
Conversation
|
@carlosms do you think we should add information about scalafmt/plugin in CONTRIBUTING.md? |
| ``` | ||
| make format-scala | ||
| ``` | ||
|
|
There was a problem hiding this comment.
I suggest we
- add details on how to configure IDE plugin with the same set of formater/liner ruler here as well
- make sure that after those steps re-formatting between CLI and IDE does not happen
build.sbt
Outdated
| @@ -38,13 +37,19 @@ assemblyMergeStrategy in assembly := { | |||
| } | |||
|
|
|||
| assemblyShadeRules in assembly := Seq( | |||
| ShadeRule.rename("com.google.common.**" -> "com.google.shadedcommon.@1").inAll, | |||
| ShadeRule | |||
There was a problem hiding this comment.
changes like this (and all below) hurt readability, are not useful and shall not be enforced by our formatter
| testOptions in Test += Tests.Argument(TestFrameworks.ScalaTest, "-l", "Cassandra") | ||
| testOptions in Test += Tests.Argument( | ||
| TestFrameworks.ScalaTest, | ||
| "-l", |
There was a problem hiding this comment.
same as above - this is not usefull
There was a problem hiding this comment.
@bzz from my understanding scalafmt tries to do the same what prettier did for javascript. Yes, it doesn't always make useful changes but consistent.
They have very good explanation about it in "why" section of readme: https://prettier.io/docs/en/why-prettier.html
I personally 100% in to sacrifice readability in exchange to consistent code style. (although about readability some people can disagree with you)
There was a problem hiding this comment.
Consistency is 👍 but autoformatter shall adhere the language styleguide and make things easier to read and maintain, not harder.
If we get back to discussing particular case at hand: following a common sense and the styleguide http://docs.scala-lang.org/style/indentation.html#methods-with-numerous-arguments it seems to me that this change should not happen.
Or did I miss something here?
There was a problem hiding this comment.
I'm not sure if it's the case here. But most probably it is. The line is more than 80 character in length. Most probably it's a reason to break arguments to multiple lines. Prettier and elm-fmt work like that.
In short, formatter can't know how to break a long line in the most readable way (it is subjectively). So it just puts each argument on new line.
There was a problem hiding this comment.
PS.
easier to read and maintain
is also subjectively. I remember a huge holywar about new elements on new line in a list.
Something like:
a = [1,2]
vs
a = [
1,
2,
]
I think the first variant is better, but surprisingly most of people in the issue voted for the second one.
| def findAllDuplicateBlobHashes( | ||
| conn: Session, | ||
| keyspace: String | ||
| ): Iterable[DuplicateBlobHash] = { |
There was a problem hiding this comment.
changes like this, that make concise code much longer, shall not be done by a formatter
|
Readme updated with IntelliJ instructions. About the style, I couldn't find any other scalafmt.conf widely used by the community, like airbnb's style guide for JS, or pep8 for python. |
|
@carlosms thank you! Seems like a rebase/reformat might be in order. Could you please verify that this autoformater respects the convention from http://docs.scala-lang.org/style/ ? I.e for http://docs.scala-lang.org/style/indentation.html#methods-with-numerous-arguments
that indentation is not applied in method call, but only for >5 arguments, etc. |
Signed-off-by: Carlos Martín <carlos.martin.sanchez@gmail.com>
Signed-off-by: Carlos Martín <carlos.martin.sanchez@gmail.com>
Signed-off-by: Carlos Martín <carlos.martin.sanchez@gmail.com>
|
Branch is now rebased. About wrapping method invocation onto multiple lines, as you can see here the arguments are formatted to one per line when the line is too long, but here (Gemini.scala:79) the method is left in one line because it fits. |
|
@carlosms could you please brake this great work down to two new PRs:
As soon as autoformat follows acceptable coding conventions from http://docs.scala-lang.org/style/ we will re-format the code. |
|
Awesome, thank you! Closing in favor of 2 smaller PRs. |
Fixes #50 & #49.
The main commit is 16cd1d1. In it scalastyle is removed, and the neo-sbt-scalafmt sbt plugin is added. There are 2 new commands to check and fix formatting,
make lint-scalaandmake format-scala. Travis will use this new lint.There is an official plugin for IntelliJ, and it can be configured to format code con save.
In 4216c9a the current code is automatically formatted with scalafmt.