-
Notifications
You must be signed in to change notification settings - Fork 122
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
Add Scala Native as a supported platform #238
Conversation
3e3c4f6
to
e6cab6f
Compare
Wow this is great. I'm not too familiar with |
Not at the moment; Scala Native doesn't have support for running tests from SBT, and ScalaTest and ScalaCheck are not available there yet. So the support is very experimental. |
@@ -125,5 +125,16 @@ package object squants { | |||
* @param d Double number to be formatted | |||
* @return | |||
*/ | |||
private[squants] def crossFormat(d: Double): String = if (d.toLong == d) { "%.1f".format(d) } else { d.toString } | |||
private[squants] def crossFormat(d: Double): String = { |
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.
On scala.js
we can put different implementations of a class for each platform under the js
and jvm
folders. Can we do the same and put the native specific code is a separate location?
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 added a Platform
object that contains platform-specific implementations of crossFormat
e6cab6f
to
9f01b17
Compare
@@ -9,6 +9,7 @@ | |||
package squants.thermal | |||
|
|||
import squants._ | |||
import squants.Platform.crossFormat |
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.
Interesting, is this really the only place this is being used? Regardless I like this solution
This is looking good and I'm all for including this new capability it but I'll wait a while for other members to chime in |
This looks fine to me, and I'm glad can isolate scala native code in its own directory. |
``` | ||
sbt +publishSigned | ||
sbt +squantsJVM/publishSigned |
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.
Is it necessary to break these out into three commands now?
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.
That's a good point, breaking it will probably slow down travis a bit while warming up sbt
@paxelord Let us know if you can change the travis task split and if you want you can add yourself to the contributors list on the README If you don't want to change those we can merge and change it later Thanks a lot for this, I'll start adding scala-native support to my own projects |
I split up the Travis task because Scala Native only supports Scala 2.11, so we cannot run the cross-version commands for native compilation. As soon as support is added in Scala Native for 2.10 and 2.12 the change can be reverted run cross-version commands for all platforms. Also, just a little confusion, @paxelord is another member on the same robotics team as me (@Team846). But I pinged him to let him know about the contributors list since he contributed additional angular units in a previous PR. |
This adds cross-compilation of Squants to Scala Native (for Scala 2.11 only). There is a small hack in
toString
forQuantity
to work around string formatting being unimplemented in Scala Native. This also updates publishing instructions to handle Scala Native only supporting Scala 2.11.