search

typelevel / algebra

Experimental project to lay out basic algebra type classes
https://typelevel.org/algebra/
Other
378 stars 69 forks source link

Add scala.js support #73

Closed ghost closed 9 years ago

ghost commented 9 years ago

Adds sjs support

non commented 9 years ago

Thanks! This looks really good.

What are the OBSOLETE files all about? Should they be removed?

ghost commented 9 years ago

Yes. I just chatted on gitter, they should go. I'll wait for any other review feedback and fix in one go

tixxit commented 9 years ago

:clap:

ghost commented 9 years ago

Also, I should add sbt-sonatype, effectively merging in https://github.com/non/algebra/pull/72

non commented 9 years ago

Sure, if you want to merge it (and update it to your working code) then go for it. Thanks!

non commented 9 years ago

So here are some review comments:

  1. Rather than RInt.apply I'd prefer something like Platform.rint to make it a bit clearer what is going on. That would also mean we could put future JS/JVM split code into Platform too.
  2. What was the rationale for changing TypeTag to TypeTagM? Are there problems with type tags in scala.js? Was there a 2.10/2.11 compatibility problem we hadn't noticed? Something else?

Other than that, this looks great. Thanks!

ghost commented 9 years ago
  1. Name change is fine. rint will be fixed in next sjs release ( probably by me) - https://github.com/scala-js/scala-js/issues/1797, but a general place holder such as Platform is a good idea eg - scalacheck has one too, and better still, private
  2. TypeTag is runtime reflection, so is not/will not be in scala.js, hence the macro implementation.
ghost commented 9 years ago

Also, there are a few dangling white spaces that I'll remove, and some documentation for the macros, platform diffs and so on

ghost commented 9 years ago

Done - also moved SerializableRules to Platform

non commented 9 years ago

I'm not sure why Travis failed, but this looks good to me. :+1:

ghost commented 9 years ago

Sbt has fixed their URL issue, can someone with access re-run the tests, please?

non commented 9 years ago

Tests pass, look great. :+1: