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

Configure scalafix #20

Merged
merged 5 commits into from
Jul 2, 2022
Merged

Configure scalafix #20

merged 5 commits into from
Jul 2, 2022

Conversation

iRevive
Copy link
Contributor

@iRevive iRevive commented Jun 19, 2022

scalafix offers handy lints and rewrites. The majority of typelevel projects use it.

build.sbt Outdated
cond = Some(s"matrix.java == 'temurin@8' && matrix.scala != '$Scala3'")
)

scalafixStep +: (ThisBuild / githubWorkflowBuild).value
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can be simplified after typelevel/sbt-typelevel#306

Copy link
Member

Choose a reason for hiding this comment

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

I thought about this a bit and have also been following the discussion in #3.

It seems like this project will likely have separate Scala 2 and Scala 3 sources. Therefore I propose the right thing to do here is to have two separate scalafix configurations, .scalafix-2.conf and .scalafix-3.conf. That way only the supported rules can be enabled in the scalafix-3.conf.

Copy link
Member

@armanbilge armanbilge Jun 19, 2022

Choose a reason for hiding this comment

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

scalafixAll requires an expensive compile step, so it should be done before after the fast header/formatting checks.

build.sbt Outdated
ThisBuild / scalaVersion := Scala213 // the default Scala

lazy val root = tlCrossRootProject.aggregate(core, java)
ThisBuild / scalafixDependencies += "com.github.liancheng" %% "organize-imports" % "0.6.0"
ThisBuild / semanticdbEnabled := !tlIsScala3.value
Copy link
Member

Choose a reason for hiding this comment

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

This will cause problems with metals which relies on semantic db.

build.sbt Outdated
Comment on lines 32 to 36
val scalafixStep = WorkflowStep.Sbt(
List("root/scalafixAll --check"),
name = Some("Check that scalafix has been run"),
cond = Some(s"matrix.java == 'temurin@8' && matrix.scala != '$Scala3'")
)
Copy link
Member

Choose a reason for hiding this comment

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

root/scalafixAll will run for all modules. scalafixAll will run for only the current active project, which in CI is either rootJVM or rootJS, which is more efficient.

build.sbt Outdated
cond = Some(s"matrix.java == 'temurin@8' && matrix.scala != '$Scala3'")
)

scalafixStep +: (ThisBuild / githubWorkflowBuild).value
Copy link
Member

@armanbilge armanbilge Jun 19, 2022

Choose a reason for hiding this comment

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

scalafixAll requires an expensive compile step, so it should be done before after the fast header/formatting checks.

build.sbt Outdated
Comment on lines 20 to 23
ThisBuild / semanticdbEnabled := true
ThisBuild / semanticdbVersion := scalafixSemanticdb.revision

ThisBuild / tlCiScalafixCheck := true
Copy link
Member

Choose a reason for hiding this comment

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

At this point you can remove these settings and just use sbt-typelevel-scalafix instead :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed. Looks much better now, thanks 🙂

@iRevive iRevive force-pushed the setup-scalafix branch 2 times, most recently from 3e0600e to 41116ee Compare June 19, 2022 16:20
iRevive added 4 commits July 2, 2022 19:51
# Conflicts:
#	java/src/main/scala/org/typelevel/otel4s/java/impl.scala
# Conflicts:
#	core/src/main/scala/org/typelevel/otel4s/otel4s.scala
#	java/src/main/scala/org/typelevel/otel4s/java/impl.scala
@iRevive iRevive merged commit 45414d3 into typelevel:main Jul 2, 2022
@iRevive iRevive deleted the setup-scalafix branch July 2, 2022 17:20
chr12c pushed a commit to chr12c/otel4s that referenced this pull request Jun 26, 2023
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.

3 participants