Support scala-native 0.4 #3750
Support scala-native 0.4 #3750
Conversation
|
discipline-munit 1.0.5 release was successful 🎉 |
|
Looks like the current build errors are specifically related to scoverage/sbt-scoverage#319 / sbt/sbt#6251 One solution I can think of (other than waiting for scoverage/sbt-scoverage#319 to be resolved) is splitting the coverage tasks into their own CI steps and temporarily disabling coverage / coverageReport for Scala 2.12.13. @larsrh Any thoughts on this? |
arashi01
force-pushed
the
scala-native-support
branch
from
be133b2
to
2f17b84
Compare
Codecov Report
@@ Coverage Diff @@
## master #3750 +/- ##
==========================================
- Coverage 90.09% 89.85% -0.25%
==========================================
Files 390 389 -1
Lines 8898 8870 -28
Branches 267 263 -4
==========================================
- Hits 8017 7970 -47
- Misses 881 900 +19 |
arashi01
force-pushed
the
scala-native-support
branch
from
e63cd40
to
df63b59
Compare
|
Solution, if acceptable, was to move coverageReport and associated prep tasks out of the matrix build and into separate job. This is arguably a cleaner solution than including coverage generation in Edit: Previous behaviour also had coverage run and reports uploaded for every JDK configured in the main build matrix. If this is actually desired, additional JDKs can be added to coverage job settings. |
|
@larsrh I think we're ready to go on this. Let me know if anything needs adjusting. |
| class FutureSuite extends CatsSuite { | ||
| val timeout = 3.seconds | ||
|
|
||
| // TODO: We shouldn't do this! See: https://github.com/scala-js/scala-js/issues/2102 |
There was a problem hiding this comment.
Is this still accurate for Scala Native?
There was a problem hiding this comment.
It seems so. Using global ExecutionContext resulted in intermittent test failures.
The actual test suite is copied from the JVM platform project, modified with with synchronous ExecutionContext replacing the global import.
arashi01
force-pushed
the
scala-native-support
branch
2 times, most recently
from
791797a
to
bbbb779
Compare
|
Sorry to rain on your parade, but we might kill coverage entirely. PR incoming, which we'll discuss first. |
No problem at all. Will rebase again when necessary! |
|
@arashi01 FYI, we've removed coverage from the build now. |
arashi01
force-pushed
the
scala-native-support
branch
2 times, most recently
from
58ea45b
to
5739d58
Compare
|
@larsrh Done. Let me know if any changes are needed. |
arashi01
force-pushed
the
scala-native-support
branch
from
5739d58
to
2fc4504
Compare
|
@larsrh I've removed all build related changes not related to adding Scala Native to build. Can make a separate pull request for build cleanup if necessary. Probably the more correct thing to do anyway! |
Sounds like a good plan. I'd like to solicit some reviews from others (also, take a closer look myself) before asking you to do even more work though 😉 But regardless, thanks already for this effort. It's nice to see that it doesn't require that many code changes. |
|
... and we have lift-off 🚀 Thanks a ton @arashi01. This will be included in the upcoming 2.4.0 release. Independently of that, if you're so inclined, it'd be great if you could file a follow-up PR with the build changes. |
Okay great! And will do! |
Requires typelevel/discipline-munit#73 and discipline-munit to be published for scala-native first for builds to go green. (Tested locally)Reorganises build to split coverage job out of main build matrix.