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

Migrate to sbt-typelevel-ci-release #522

Merged
merged 5 commits into from
Mar 6, 2022

Conversation

armanbilge
Copy link
Member

So the tricky thing about this is change is it seems that MiMa is enabled for the first time in this project, and there are many many issues.

Apparently master has already broken binary-compatibility compared to the latest release 1.8.3 due to the changes in #509. This is not considering the 100+ MiMa binary issues if we compare against 1.0.0 :)

One easy strategy may be to declare a clean slate: your next version should be 2.0.0 since it's binary-breaking, and from then on you will preserve binary compatibility (or bump major version again).

WDYT?

@cquiroz
Copy link
Collaborator

cquiroz commented Mar 6, 2022

I'm pretty sure binary compatibility has been broken
Bumping to 2.0 and starting with a clean sheet seems the best option

@armanbilge
Copy link
Member Author

armanbilge commented Mar 6, 2022

👍 that's easy enough :)

Btw, if you've been following the discussion, we need to remove the repo secrets from squants and also spire, thanks!
https://github.com/orgs/typelevel/teams/maintainers/discussions/6

@cquiroz
Copy link
Collaborator

cquiroz commented Mar 6, 2022

Yes go ahead, or I can do it if I have the permission

@armanbilge
Copy link
Member Author

I don't have permissions, so it will have to be you :)

@armanbilge
Copy link
Member Author

Native fails with this:

[info] - should return Length when cube rooted *** FAILED ***
[info]   3.0000000000000004 m was not equal to 3.0 m (VolumeSpec.scala:170)

@armanbilge
Copy link
Member Author

It fails locally for me too ... were you not running this test before?

@armanbilge
Copy link
Member Author

Ah, you were only compiling native but not testing.

sbt +squantsJVM/test +squantsJS/test +squantsNative/compile

@cquiroz
Copy link
Collaborator

cquiroz commented Mar 6, 2022

I think it wasn't possible to compile to native back then, maybe it is now

@cquiroz
Copy link
Collaborator

cquiroz commented Mar 6, 2022

Ok, I removed the secrets from both spire an squants

CubicMeters(27).cubeRoot should be(Meters(3))
CubicMeters(64).cubeRoot should be(Meters(4))
Copy link
Member Author

Choose a reason for hiding this comment

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

Easy fix

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ideally you could choose the number type to have exact calculations but squants only supports doubles

@armanbilge
Copy link
Member Author

Good to go, but you will need to remove the old status check.

Copy link
Collaborator

@cquiroz cquiroz left a comment

Choose a reason for hiding this comment

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

LGTM, thankks a lot

@cquiroz cquiroz merged commit 22f0fb0 into typelevel:master Mar 6, 2022
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.

2 participants