Skip to content

Conversation

@taisukeoe
Copy link
Contributor

@taisukeoe taisukeoe commented Jun 28, 2020

Hi, thank you for maintaining Scalafix rules to migrate easier.

I'd send this PR because:

So Scala library maintainers start trying to use ExplicitNonNullaryApply in their Scala 2.13 projects. Since its migration rule Scala version support is not obvious, it would be pitfall for them.

IMHO it should be explicitly stated in README now, and can be removed right after Scalafix next version including cross-compiled rules support is released.

Your feedback is welcome.

make scalafix version consistent
@taisukeoe taisukeoe force-pushed the add-scala-version-caution branch from aaca9bd to 7b96e29 Compare June 29, 2020 08:39
2. Add SemanticDB: `addCompilerPlugin(scalafixSemanticdb)`
3. Configure Semantic DB: `scalacOptions ++= List("-Yrangepos", "-P:semanticdb:synthetics:on")`
4. Run the rule: `scalafix dependency:Scala213@org.scala-lang:scala-rewrites:0.1.0-SNAPSHOT`
5. Run the rule on test sources: `Test/scalafix dependency:Scala213@org.scala-lang:scala-rewrites:0.1.0-SNAPSHOT`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Scala213 rule has been renamed and no longer exists.

@SethTisue SethTisue removed their request for review June 29, 2020 19:14
@@ -1,3 +1,7 @@
# CAUTIONS!
Current released Scalafix supports ExplicitNonNullaryApply [ONLY in Scala 2.12](https://github.com/scala/scala-rewrites/issues/31).
Scalafix will support 2.13 and 2.11 with cross-building ExplicitNonNullaryApply rule against them.

Choose a reason for hiding this comment

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

As someone who doesn't quite understand the mechanics of these libraries properly, allow me to attempt to clarify.

"Scalafix v1 will allow the ExplicitNonNullaryApply rule to work and fix up code in Scala 2.11 and 2.13"

Is that correct?

Copy link
Contributor Author

@taisukeoe taisukeoe Jun 30, 2020

Choose a reason for hiding this comment

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

Thank you for clarification.

Is that correct?

It looks good to me except Scalafix version, depending on Scalafix release plans.

I'm not sure it will be effective only after Scalafix v1, since the PullRequest has been already merged and there might be additional v0.9.X releases.

So I'm fine with rewriting the sentence with:

"Scalafix will allow the ExplicitNonNullaryApply rule to work and fix up code in Scala 2.13"

(Considering the nature of the rule, 2.11 can be omitted)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Of note, even coming Scalafix releases, ExplicitNonNullaryApply rule has to be cross-built by #31 PR.

Copy link
Contributor

@bjaglin bjaglin Jul 1, 2020

Choose a reason for hiding this comment

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

although it won't be automatic, the upcoming 0.9.18 release of sbt-scalafix (probably out tomorrow) will support running rules compiled in 2.11 and 2.13, see scalacenter/scalafix#1166 for needed steps (feedback welcome there too!)

@dwijnand dwijnand merged commit 8de8ec7 into lightbend-labs:master Jun 30, 2020
@taisukeoe taisukeoe deleted the add-scala-version-caution branch July 1, 2020 03:32
@taisukeoe
Copy link
Contributor Author

@dwijnand Thank you for merging!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants