asynkron / protoactor-kotlin

Ultra-fast distributed cross-platform actor framework
http://proto.actor
Apache License 2.0
221 stars 25 forks source link

Kotlin upgrade fix #47

Closed james-cobb closed 5 years ago

james-cobb commented 5 years ago

These look to me to be the only changes needed to fix the kotlin version upgrade.

orjan commented 5 years ago

I'm not sure why the feedback from travis is not working, but it looks like the build is failing: https://travis-ci.org/AsynkronIT/protoactor-kotlin/builds/509961232

codecov-io commented 5 years ago

Codecov Report

Merging #47 into master will decrease coverage by 1.29%. The diff coverage is 0%.

Impacted file tree graph

@@             Coverage Diff             @@
##             master      #47     +/-   ##
===========================================
- Coverage     30.01%   28.72%   -1.3%     
+ Complexity      186      184      -2     
===========================================
  Files            50       52      +2     
  Lines          1749     1744      -5     
  Branches        278      282      +4     
===========================================
- Hits            525      501     -24     
- Misses         1129     1145     +16     
- Partials         95       98      +3
Impacted Files Coverage Δ Complexity Δ
...or/src/main/kotlin/actor/proto/java/ContextImpl.kt 0% <0%> (ø) 0 <0> (ø) :arrow_down:
...roto-actor/src/main/kotlin/actor/proto/java/API.kt 0% <0%> (ø) 0 <0> (ø) :arrow_down:
.../kotlin/actor/proto/router/BroadcastRouterState.kt 50% <0%> (-50%) 5% <0%> (+1%)
proto-actor/src/main/kotlin/actor/proto/API.kt 66.66% <0%> (-33.34%) 0% <0%> (ø)
...kotlin/actor/proto/router/RoundRobinRouterState.kt 44.44% <0%> (-11.12%) 3% <0%> (ø)
...ain/kotlin/actor/proto/router/RandomRouterState.kt 40% <0%> (-10%) 3% <0%> (ø)
...in/actor/proto/router/ConsistentHashRouterState.kt 45.45% <0%> (-4.55%) 4% <0%> (ø)
proto-actor/src/main/kotlin/actor/proto/Props.kt 96% <0%> (-4%) 11% <0%> (ø)
...r/src/main/kotlin/actor/proto/AllForOneStrategy.kt 52.38% <0%> (-2.17%) 6% <0%> (ø)
...r/src/main/kotlin/actor/proto/OneForOneStrategy.kt 68.42% <0%> (-1.58%) 8% <0%> (ø)
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 28a7799...39f6430. Read the comment docs.

james-cobb commented 5 years ago

There was another dependency that was missing, and then finally the intermittent test failed again, so I've cherry picked the fix from #44 and added to this PR.

Now this is passing - I think it's the minimal patch to get master branch working again.

Globegitter commented 5 years ago

@rogeralsing would also be great to get this PR merged if nothing is blocking it anymore.

guenhter commented 5 years ago

@james-cobb could you resolve the conflicts

james-cobb commented 5 years ago

@guenhter Yes I'll resolve the conflicts so we can merge this too. On vacation but will find some time when I can... :)

james-cobb commented 5 years ago

@guenhter conflicts resolved now. This is now a pretty simple diff and gets master building with all tests passing with the new coroutines API. @rogeralsing would be great to get this PR merged now.

guenhter commented 5 years ago

@james-cobb cool. Maybe you squash some of the commits. 9 commits seems like a little bit overkill for the changes done here, doesn't it?

james-cobb commented 5 years ago

@guenhter that's a good call. I've squashed down to one commit for the original changes, and one for merging master to sort the conflicts.

guenhter commented 5 years ago

@rogeralsing Thx for the invite to the organization. It seems that I have now right e.g. to merge in the protoactor-go but in the protoactor-kotlin I can't to anything developer related... Was your intention when gave us developer rights to support you on merging requests and having an eye on issues? If so, maybe you could have a second look over the kotlin project right. Thx

rogeralsing commented 5 years ago

It was an oversight on my side, I added the Kotlin repo to the developers team now. this should fix the issue :-)

guenhter commented 5 years ago

Interestingly, it's still not possible for me to merge e.g. this merge request...

guenhter commented 5 years ago

Nice. Now it works. Thx