-
Notifications
You must be signed in to change notification settings - Fork 185
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
Ondemand compilation doesn't take scalacOptions #1215
Conversation
c58b7dc
to
d3592a4
Compare
f4db66a
to
468deb8
Compare
linked to this scalameta/scalameta#2094 |
Just landed on this and trying to understand the status: |
So if we pass the scalacOptions, it fixes the onDemand compilaton for 2.11 and 2.12, but not 2.13. So the bug is maybe on the reporter part. The bug is visible on scalameta too. |
Oh ok. I guess that's still something good to take in then (by removing tests for 2.13), and follow up 2.13 in another PR? |
0c4e8e8
to
e52b0f2
Compare
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 for making this incremental!
val ss = new Settings() | ||
val command = new CompilerCommand(scalacOptions, ss) | ||
val settings = command.settings | ||
settings.YpresentationAnyThread.value = true | ||
settings.classpath.value = validatedClasspath.syntax | ||
val reporter = new StoreReporter() |
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.
out of curiosity: is this block evaluated lazily on purpose to capture some state that it not available eagerly?
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.
So the creation of Global was lazy, I just put the entire block as lazy.
Usually, if the semanticdb file is present, global is never instantiated.
I don't think there is any state that is captured later..
test("on-demand with scalacOptions") { | ||
// Todo: Ondemand compilation doesn't take scalacOptions or the reporter doesn't gather all warns in 2.13 | ||
// see https://github.com/scalacenter/scalafix/pull/1215 | ||
if (ScalaVersions.isScala213) true |
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 we open a separate issue to track that?
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.
ok
No description provided.