-
Notifications
You must be signed in to change notification settings - Fork 17
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
Pass scalaVersion
to Scalafix as needed for Scalafix 0.13.0
#205
Conversation
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.
Thanks! Just some minor comments but LGTM overall 👍
@@ -63,6 +66,7 @@ object ScalafixModule { | |||
os.pwd | |||
) | |||
|
|||
@nowarn("msg=parameter scalaBinaryVersion in method fixAction is never used") |
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.
I assume you left the parameter for binary compatibility in the patch line? Nitpicking: we could create an overload (easier to clear later) and get rid of the propagation of scalafixScalaBinaryVersion
for the real code path.
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.
👍
PathRef(sources().head.path / "fix-2.12") -> Seq( | ||
TestInvocation.Targets(Seq("__.fix")), | ||
TestInvocation.Targets(Seq("verify")) | ||
), |
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.
I just saw tests were failing because of that boyscout-rule addition, independently of the actual purpose of this PR 🤔
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.
The tests were not running. I could do this in a separate PR but honestly I don't see the reason.
Co-authored-by: Brice Jaglin <bjaglin@gmail.com>
Co-authored-by: Brice Jaglin <bjaglin@gmail.com>
scalaVersion
to Scalafix as needed for Scalafix 0.13.0
scalaVersion
to Scalafix as needed for Scalafix 0.13.0scalaVersion
to Scalafix as needed for Scalafix 0.13.0
scalafixScalaBinaryVersion
in favor ofscalaVersion
fixAction
overloads which take scalaBinaryVersion as a parameterPull Request: #205