-
Notifications
You must be signed in to change notification settings - Fork 44
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
Support scalafixDependencies #9
Conversation
This PR improves support for custom rules that are published to Maven Central. This functionality is required for rules like scala-collection-compat. We do a best-effort to avoid redundant dependency fetching of `scalafixDependencies` by caching the results in an in-memory map. In the future, it would make sense to expose more powerful APIs in scalafix to cache classloaders between invocations.
build.sbt
Outdated
@@ -20,7 +20,7 @@ developers := List( | |||
resolvers += Resolver.sonatypeRepo("releases") | |||
libraryDependencies ++= List( | |||
"org.eclipse.jgit" % "org.eclipse.jgit" % "4.5.4.201711221230-r", | |||
"com.geirsson" %% "coursier-small" % "1.0.0-M2", | |||
"com.geirsson" %% "coursier-small" % "1.0.0-M4", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a comment why we need this. GitHub repo link. I guess it's coursier 1.0.0-M4. Why not the latest version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, added a comment.
Right(ScalafixInterface.classloadInstance()) | ||
val dep = new Dependency( | ||
"ch.epfl.scala", | ||
s"scalafix-cli_${BuildInfo.scala212}", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if we keep the "g" % "a" %% "v"
syntax and assert that the scalaVersion == BuildInfo.scala212 ? To me this feels more natural, because I always use %%
for scala library.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not the syntax for scalafixDependencies
, this is the coursier-small API that users won't observe
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See basic/build.sbt
for an example scalafixDependencies
usage
), | ||
scalafixDependencies := List( | ||
// Custom rule published to Maven Central https://github.com/olafurpg/example-scalafix-rule | ||
"com.geirsson" % "example-scalafix-rule_2.12" % "1.1.1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be "com.geirsson" %% "example-scalafix-rule" % "1.1.1"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That won't work for the scripted tests on 0.13
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair point.
Fixes #7.
This PR improves support for custom rules that are published to Maven
Central. This functionality is required for rules like
scala-collection-compat. We do a best-effort to avoid redundant
dependency fetching of
scalafixDependencies
by caching the resultsin an in-memory map. In the future, it would make sense to expose more
powerful APIs in scalafix to cache classloaders between invocations.