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.

Sign up for GitHub

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

Jump to bottom

Support Scala 2.13 #55

Merged
merged 5 commits into from
Jul 1, 2019
Merged

Support Scala 2.13 #55

merged 5 commits into from
Jul 1, 2019

Conversation

kevinwright
Copy link
Contributor

@kevinwright kevinwright commented Jun 26, 2019 •
edited
Loading

Draft PR for review only... the drone build and .travis.yml still need updating to support the changes.

tindzk
tindzk reviewed Jun 27, 2019
View reviewed changes
build.sbt Outdated
excludeFilter in Test := "*",
val JsSettings = JvmSettings ++ nocomma {
// From https://github.com/scala-js/scala-js/pull/2954
libraryDependencies := libraryDependencies.value.filterNot(_.name == "scalajs-compiler")
Copy link
Member

Choose a reason for hiding this comment

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

This line should only be needed for < 2.13.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I was considering that, planned on revisiting alongside travis

@tindzk
Copy link
Member

tindzk commented Jun 27, 2019

Thanks a lot! The changes look good so far. I will look into the Travis CI failure on the weekend and release a new version once everything works as expected. As for the Drone CI configuration, I will open a separate pull request since Seed does not support 2.13 yet.

@kevinwright
Copy link
Contributor Author

kevinwright commented Jun 28, 2019 •
edited
Loading

Turns out that this wasn't picking up the correct source directories, now fixed. Also added a bunch of de-linting and other warning options for scalac.

The main outstanding issue now seems to be an error in scalaJS tests under 2.11 and 2.12 (2.13 works fine, which is odd, given that this is the new one!)
Specifically:

org.scalajs.testcommon.RPCCore$RPCException:
  scala.scalajs.js.JavaScriptException:
    TypeError:
      Cannot read property 'NodePropSpec' of undefined

@tindzk
Copy link
Member

tindzk commented Jun 29, 2019

The clean-up looks good. I tried the branch and figured out the problem with the test failures. This line got lost in the refactoring:

addCompilerPlugin("org.scala-js" % "scalajs-compiler" % scalaJSVersion cross CrossVersion.patch)

When I added it back after libraryDependencies := libraryDependencies.value.filterNot(_.name == "scalajs-compiler"), the tests started to run again for < 2.13.

@kevinwright
Copy link
Contributor Author

Good catch, you have sharper eyes than mine :)

@kevinwright kevinwright force-pushed the feature/scala-2.13 branch 3 times, most recently from 766d1e7 to 979add6 Compare June 29, 2019 15:59
@kevinwright
Copy link
Contributor Author

And... travis is passing. Just the source dirs to fix up for drone now

tindzk
tindzk reviewed Jun 29, 2019
View reviewed changes
build212.toml Show resolved Hide resolved
tindzk
tindzk reviewed Jun 29, 2019
View reviewed changes
version.sbt Outdated Show resolved Hide resolved
@kevinwright
Copy link
Contributor Author

Hot damn, it's passing!
I'll add a build213.toml and take it out of draft

@kevinwright kevinwright marked this pull request as ready for review July 1, 2019 10:25
kevinwright and others added 2 commits July 1, 2019 11:32
@kevinwright
Added 2.13 to README
88040bf
@tindzk
README: Fix typo
cb73fb1 
Also add Kevin Wright as contributor.
@tindzk
Copy link
Member

tindzk commented Jul 1, 2019

Thanks for your hard work! I will go ahead and merge it. The new version should go out tomorrow.

@tindzk tindzk merged commit c4f7b15 into sparsetech:master Jul 1, 2019
@kevinwright kevinwright deleted the feature/scala-2.13 branch July 1, 2019 14:12
@tindzk
Copy link
Member

tindzk commented Jul 6, 2019

Sorry, I could not release the version yet since we are blocked by sbt/sbt-projectmatrix#18.

tindzk added a commit that referenced this pull request Jul 12, 2019
@tindzk
Build: Add missing Scala Native compiler plug-in
9e061ea 
Fixes regression introduced in #55.

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

@tindzk tindzk tindzk approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@kevinwright @tindzk