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.

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

Cross-publish core module #157

Merged
merged 28 commits into from
Nov 18, 2020
Merged

Cross-publish core module #157

merged 28 commits into from
Nov 18, 2020

Conversation

jisantuc
Copy link
Contributor

@jisantuc jisantuc commented Oct 17, 2020

Overview

This PR cross-publishes core for Scala.js. Or, you know, it will eventually.

Checklist

  • New tests have been added or existing tests have been modified
  • Changelog updated

Notes

All TODOs done! 🎉

I'm pretty sure that the local publication proves that sonatype publication should Just Work ™️, but if there's other evidence to gather about that, I'm happy to gather it.

Right now I'm working on getting types lined up to have a js module that works. The main focus so far has been on adding a Geometry type that doesn't depend on GT. I'm only bothering for now with 2d points, polygons without holes, and multipolygons. To get tests running I'm also going to add some optics, because it turns out monocle publishes for Scala.js and some prisms will make my tests nicer to write. Once I have a SerDeSpec for the geometry types I'll work on actually publishing, which I don't uh technically know how to do yet (though maybe the nice sbt plugins will take care of that bit).

Next TODOs:

  • export some types to see if they end up in the fastoptjs file as i expect
  • get tests actually running
  • add node to ci environment
  • add an extent type to make geom generation less haphazard

Update 2020/11/11: I'm working now on porting everything that depends on java.time into the JS side. This library makes it look like I can add a dep and then just use the same java.time API, but I'm getting a lot of complaints in test runs about Referring to non-existent class java.time.format.DateTimeFormatter and other classes, so I'm suspicious. I think the JS side is just going to need to be more ad hoc about time types for now. You can see failures in this build output.

Next TODO:

  • move ForeignImplicits out of shared SerDeSpec because I don't understand what it will pull in there (I assume the cross-project appropriate package?)
  • choose a time library, like https://github.com/zoepepper/scalajs-jsjoda maybe scala-java-time works fine as long as it's declared as a dep not only in both core and coreTest 🎉

Testing

  • run test from root and see that you get all the tests from JVM and JS sides
  • run publishLocal from root and see that you publish both JVM and JS sides to local cache

Closes jisantuc/fp-http-azavea-rd#3

@jisantuc jisantuc force-pushed the feature/js/cross-publish-core branch from 468b0f1 to 2c690dc Compare October 30, 2020 22:52
@jisantuc jisantuc force-pushed the feature/js/cross-publish-core branch from 21c88fc to 7c9c94d Compare November 11, 2020 22:26
@jisantuc jisantuc force-pushed the feature/js/cross-publish-core branch from 4df5c47 to f2bcad4 Compare November 16, 2020 22:46
@jisantuc jisantuc changed the title [wip] Cross-publish core module Cross-publish core module Nov 16, 2020
Copy link
Collaborator

@pomadchin pomadchin left a comment

Choose a reason for hiding this comment

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

Project works and this is a nice change towards JS support 🎉

My main concerns are only about the code duplication. Is it possible to avoid it as much as it is possible? Theoretically we could only try to substitude Geometry and that can solve all JS compilation issues, rather than creating lots of classes duplicates. It is also possible to move specific to platform methods into syntax extensions.

@jisantuc have you considered that? do you know is it possible?

@jisantuc jisantuc merged commit e3421d9 into master Nov 18, 2020
@jisantuc jisantuc deleted the feature/js/cross-publish-core branch November 18, 2020 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add hand-rolled JS geometry types to stac4s
2 participants