-
Notifications
You must be signed in to change notification settings - Fork 32
Add Scala.js cross build #35
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
Conversation
5e60c1e
to
a14f62b
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.
the build.sbt
changes LGTM
I don't think the .travis.yml
matrix is right; I looks to me like it currently results in only JS getting built/tested, JVM is skipped. see the matrix:
section here: https://github.com/scala/scala-parser-combinators/blob/1.2.x/.travis.yml
(sorry, needs rebase because of #39) |
@SethTisue Rebased and followed the convention in |
build.sbt
Outdated
.aggregate(`scala-collection-contribJS`, `scala-collection-contribJVM`) | ||
.settings(disablePublishing) | ||
|
||
lazy val `scala-collection-contrib` = crossProject(JVMPlatform, JSPlatform) |
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.
May I suggest using a CrossType.Pure
project type?
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 tried, but encountered test compilation error during root/test
, though fine with individual scala-collection-contribJ(S|VM)/test
.
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.
Fixed and now Pure
.
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.
Weirdly, it seems that e15c87d has been overridden by another commit.
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.
Sorry, it was my mistake.
test
or root/test
still causes error as mentioned above, so removed the commit.
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.
Indeed, I’ve just tried and observed the same behavior as you. Any idea, @SethTisue or @sjrd? (I guess the problem might come from sbt-scala-module or sbt-crossproject?). In the meantime I’ve found a workaround (see exoego#1)
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.
Relying on aggregates with a Pure cross-project in(file("."))
leads to issues, because the default root project created by sbt will also be in(file("."))
and hence will try to pick up the sources and tests of the cross-project, leading to weird issues because the root project is not configured properly (for example it will be missing dependencies).
The solution is not to rely on aggregates. Rewire the default test
command instead, or specify a project in which sbt should execute tasks by default that is different from the root project.
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.
Oh, right, I missed the fact that both projects were in(file("."))
. We should just use a different root directory for the cross project, then.
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 merged regardless, but of course a followup PR would be welcome.)
I had to add a dependency on JUnit to the root project, otherwise the aggregated projects failed compiling.
Use CrossType.Pure
Closes #30