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

Fetch more tooling deps in prepareOffine #2294

Merged
merged 1 commit into from
Jan 30, 2023

Conversation

lefou
Copy link
Member

@lefou lefou commented Jan 30, 2023

ScalaJSModule and ScalaNativeModule did not fetched their worker and tooling classpaths previously.

`ScalaJSModule` and `ScalaNativeModule` did not fetched their worker and tooling classpaths previously.
@lefou lefou requested a review from lolgab January 30, 2023 14:02
@@ -235,6 +236,17 @@ trait ScalaJSModule extends scalalib.ScalaModule { outer =>
/** Name patterns for output. */
def scalaJSOutputPatterns: Target[OutputPatterns] = T { OutputPatterns.Defaults }

override def prepareOffline(all: Flag): Command[Unit] = {
val tasks =
if (all.value) Seq(scalaJSToolsClasspath)
Copy link
Member

Choose a reason for hiding this comment

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

scalaJSToolsClasspath are not very big and necessary to do anything useful with ScalaJS (except plain compilation), so maybe they could be downloaded even without requiring the all flag.
What do you think?

Copy link
Member Author

@lefou lefou Jan 30, 2023

Choose a reason for hiding this comment

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

Yeah, makes sense. But with the same argument, we should always fetch Zinc dependencies in ScalaModule. Current scaladoc says:

/** @param all If `true` , fetches also sources, Ammonite and compiler dependencies. */

Copy link
Member

Choose a reason for hiding this comment

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

Then it agrees with current semantics 👍

@lefou lefou merged commit ac0ea4e into com-lihaoyi:main Jan 30, 2023
@lefou lefou deleted the prepare-offline branch January 30, 2023 20:10
@lefou lefou added this to the 0.11.0-M4 milestone Jan 30, 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.

2 participants