47degrees / helios

A purely functional JSON library for Kotlin built on Λrrow
https://47degrees.github.io/helios/
Apache License 2.0
169 stars 22 forks source link

Bump Arrow version 0.10.3 #120

Closed lenguyenthanh closed 4 years ago

lenguyenthanh commented 5 years ago

Close #119, I updated Arrow version 0.10.0

Everything works fine but there are more warnings when building than before. I don't know should I fix them now or do it in another PR.

I also had to update kotlintest library version 3.4.1 because tests were broken with old kotlintest version 3.1.11.

AdrianRaFo commented 4 years ago

I think this should wait until we have a kotlin test version using arrow 0.10.0 in order to avoid binary issues and this should also includes (or create an issue) the deletion of all the deprecations

lenguyenthanh commented 4 years ago

Hey @AdrianRaFo, I'll create an issue for removing all deprecation after this issue. For now, I'll try to work on bump Arrow version 0.10.0 for kotlintest.

AdrianRaFo commented 4 years ago

Awesome thanks

lenguyenthanh commented 4 years ago

I'm working on update kotlintest here: https://github.com/kotlintest/kotlintest/pull/1002

lenguyenthanh commented 4 years ago

As I understand, Arrow version 0.10.0 will be included in kotlntest 4.0.0, which may take sometime to be released.

I used dependency strategy to enforce Arrow version 0.10.0. But it still failed in benchmark test.

Octogonapus commented 4 years ago

Arrow version 0.10.0 will be included in kotlntest 4.0.0

That's right, they updated their matchers for the new package structure in arrow 0.10.0. I have been having to use a snapshot build for my projects.

lenguyenthanh commented 4 years ago

Should we use kotlintest snapshot version?

Octogonapus commented 4 years ago

Should we use kotlintest snapshot version?

I'll let the real helios maintainers make that call. Having snapshot builds in the repo is a controversial decision.

nomisRev commented 4 years ago

@lenguyenthanh @Octogonapus Since Helios hasn't had a stable release, and it's only a test dependency this is okay in my opinion.

cc\ @AdrianRaFo

i-walker commented 4 years ago

@nomisRev @AdrianRaFo won"t be available for the next 2 weeks. I would go with your opinion.

lenguyenthanh commented 4 years ago

Ok, I'll try to use kotlinttest snapshot on next few days.

nomisRev commented 4 years ago

Hey @lenguyenthanh! Anything I can do to help close this?

lenguyenthanh commented 4 years ago

Hey @nomisRev, I think the main issue with this PR is the benchmark test failed. I don't know how to fix this issue.

Other issue is upgrading to new kotlintest 4.0 snapshot which I can work on that.

nomisRev commented 4 years ago

Hey @lenguyenthanh,

If you could look into KotlinTest that be great! I can take a look into the benchmarks and figure out what's going on. Your changes look good 👍

lenguyenthanh commented 4 years ago

Great, thanks Simon!

lenguyenthanh commented 4 years ago

Hi all, I just pushed a new commit that upgrades kotlintest version 4.0.2758-SNAPSHOT.

In this commit, I removed arrow-test dependency because arrow-test depends on old kotlinttest version 3. I added, helios.arrow package in helios-test module. This package contains some classes from arrow-test module with new kotlintest dependency. This package can be removed after Arrow uses kotlintest 4.0.

cc @nomisRev

lenguyenthanh commented 4 years ago

oh, it failed with 0.10.3 :(. I'll fix it later today.

JorgeCastilloPrz commented 4 years ago

Might be good to flag those temporary test utils as @Deprecated

nomisRev commented 4 years ago

Thanks for the hard work @lenguyenthanh!

lenguyenthanh commented 4 years ago

Thanks @nomisRev!