search

Spinoco / protocol

Various protocol decoders and encoders
MIT License
28 stars 9 forks source link

Support Scala 2.13 #62

Closed jhnsmth closed 4 years ago

jhnsmth commented 5 years ago

Unfortunately, I didn't notice that kafka isn't yet published for 2.13 and according to their JIRA it scheduled for 2.4.0, so we should not expect to get it pretty soon.

I'll try to setup some cross-build stuff in a couple of days.

AdamChlupacek commented 5 years ago

@jhnsmth could you please try to get this into form where every other project than kafka compiles under 2.13? so that we are ready as we can be.

jhnsmth commented 5 years ago

@AdamChlupacek sry, couldn't find some time. I hope to do it tomorrow

AdamChlupacek commented 5 years ago

@jhnsmth This seems nearly finished now? Seems that tests are passing, weird that we do not see an issue with kafka anywhere.

jhnsmth commented 5 years ago

@AdamChlupacek Sorry, for the delay. Yes, it should be finished now. But unfortunately there are some deprecation warnings on 2.13 related to the new collections api, so fatal warnings is turned off, is it ok for now ? They can be fixed with some refactoring or by adding dependency on scala-collection-compat which I tried to avoid

AdamChlupacek commented 5 years ago

@jhnsmth Sorry for such late response, been quite busy lately. I would prefer not to turn off fatal warnings, as we are depending on them, especially since they only appear when you are recompiling the given file, as such with incremental compilation it is quite hard to maintain code for production uses as this can hide some important errors.

jhnsmth commented 5 years ago

@AdamChlupacek sry, for a long reply. I'm afraid it's not simple to enable fatal warnings for 2.13. Because of the deprecations in scala standard library. So it requires either adding dependency on https://github.com/scala/scala-collection-compat and rewriting using new syntax or breaking binary compatibility. Here is one of the examples. If I'm wrong please correct me.

I've added a commit that enables them for scala version below 2.13. But I'm open to any other suggestions

guymers commented 5 years ago

A possible solution to allow fatal warnings to be turned on for all Scala versions https://github.com/guymers/protocol/commit/00ebbb5e91765b849cce0e645ceeb971720fc85b

AdamChlupacek commented 5 years ago

@jhnsmth I quite like @guymers solution, that seems to me to be the correct way to do it, as there are reasons for why scala 2.13 deprecated some of the constructors. Is it possible to update the PR with these changes? I will in the meanwhile create series/0.4 where we can put this.

jhnsmth commented 5 years ago

@AdamChlupacek I'll do it late tonight or tomorrow. @guymers If you do not mind I'll cherry-pick your commit

guymers commented 5 years ago

I do not mind. Looking forward to seeing Scala 2.13 support, thank you for creating this PR.

If you don't mind though could you please fix the commit message: lines is deprecated in 2.13 not 2.11 and linesIterator in 2.11 not 2.13.

jhnsmth commented 5 years ago

All done! Ready for a final review!

lavrov commented 5 years ago

Can this be merged to unblock https://github.com/Spinoco/fs2-http/pull/37 ?

s5bug commented 5 years ago

@jhnsmth Any chance that, while the maintainers of fs2-http don't have 2.13 support merged in, you can publish 2.13 artifacts under your own organization so that projects can work on being migrated to 2.13?

AdamChlupacek commented 4 years ago

Sorry for such a delay with this guys, i will get this out asap.

AdamChlupacek commented 4 years ago

So i tried to do the release, but it would seem we need to have all the cross builds in the main aggregating file, as such we need to get the kafka protocol to be building for 2.13, I will get a stab at it today.