Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update scalastyle; add scapegoat and scalafmt #3356

Closed

Conversation

philvarner
Copy link
Contributor

Signed-off-by: philvarner philvarner@gmail.com

Overview

  1. refactor scala versions out into variables
  2. updates scalastyle dependency to the maintained 'beautiful-scala' package instead of the abandoned one
  3. adds scapegoat configuration for static analysis (similar to config in stac4s)
  4. add scalafmt (off by default-- maybe at some point all the code should be reformatted in one PR). The .scalafmt.conf was copied from stac4s

Checklist

  • n/a ./CHANGELOG.md updated, if necessary. Link to the issue if closed, otherwise the PR.
  • n/a Module Hierarchy updated, if necessary
  • n/a docs guides update, if necessary
  • n/a New user API has useful Scaladoc strings
  • n/a Unit tests added for bug-fix or new feature

Demo

none

Notes

none

Signed-off-by: philvarner <philvarner@gmail.com>
build.sbt Outdated
@@ -1,8 +1,9 @@
import sbt.Keys._

ThisBuild / scalaVersion := "2.12.13"
ThisBuild / scalaVersion := Version.scala211
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to see Version.scala212 as the default here. :D
2.11 is not a priority atm, more like a legacy crossompilation that I'd really like to drop.
We use GT mostly with 2.12.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry, that must have been a typo when I originally made this change -- I've been cross-compiling most of the time and didn't notice.

.scalafmt.conf Outdated
version = 2.7.5
align = more // For pretty alignment.
maxColumn = 120
newlines.alwaysBeforeTopLevelStatements = true
Copy link
Member

@pomadchin pomadchin Mar 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does scalafmtAll breaks everything? :/ I'd expect it to reformat 90% of our files.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does -- I mostly just wanted to get it in here, but have it be unused. But, I think it would be better to spend a bit of time aligning the rules with the existing code before merging. Putting PR in draft.

Copy link
Member

@pomadchin pomadchin Mar 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is this tool that hopefully can generate the scalafmt file that would fit our needs: https://github.com/tanishiking/scalaunfmt

(I haven't tried it and I would not be that positive about it), scalafmt is definitely going to break our type params with lots of imports formatting ):

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, I'll take a look at that tool. I think it's more important to be consistent within the codebase than adhere to some arbitrary external standard, so I'll try to write the rules that way.

Signed-off-by: philvarner <philvarner@gmail.com>
Signed-off-by: philvarner <philvarner@gmail.com>
@philvarner philvarner marked this pull request as draft March 11, 2021 16:40
Signed-off-by: philvarner <philvarner@gmail.com>
@philvarner philvarner force-pushed the scalastyle-scapegoat-scalafmt branch from 0843f2b to ecfef2d Compare March 12, 2021 14:20
@philvarner philvarner closed this Jun 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants