search

debasishg / scala-redis

A scala library for connecting to a redis server, or a cluster of redis nodes using consistent hashing on the client side.
1.02k stars 219 forks source link

Bump docker testkit. Remove scala 2.10 support. Add scala 3 support #296

Closed hughsimpson closed 2 years ago

hughsimpson commented 2 years ago

Hello, me again. I've updated the docker testkit library properly now, and run the tests locally. Have included the test results (added in the penultimate commit, then removed in the final one) so you can have a bit more confidence. The new docker testkit API is completely incompatible with the version published for scala 2.10 so this pr, of necessity, removes support for it.

hughsimpson commented 2 years ago

@debasishg would appreciate it if you could take another look

debasishg commented 2 years ago

@hughsimpson Thanks for the PR. Why did u have to remove the 2.10 support ? Just curious. I will take a detailed look tomorrow though and merge if it goes through.

hughsimpson commented 2 years ago

Just because docker testkit 0.11.0 isn't published for it and the API change makes it incompatible with older versions. I guess it would be possible to maintain scala 2.10 specific forks of the tests under scala_2.10 dirs and whatnot, but it's a fairly heavy bit of lifting to do and given the age of that version it doesn't feel worthwhile to me

hughsimpson commented 2 years ago

@debasishg You seeing issues with this? I found it ok when I tried locally myself... Although given my history with this repo I understand if you have reservations 😅

debasishg commented 2 years ago

I will try it out today .. apologies for the delay. Was too tied up over the last couple of days.

On Sat, 18 Dec 2021 at 4:44 AM, hughsimpson @.***> wrote:

@debasishg https://github.com/debasishg You seeing issues with this? I found it ok when I tried locally myself... Although given my history with this repo I understand if you have reservations 😅

— Reply to this email directly, view it on GitHub https://github.com/debasishg/scala-redis/pull/296#issuecomment-997084253, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAA2FX23JGMEL74WGDOKGT3URO733ANCNFSM5KAT4RDA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you were mentioned.Message ID: @.***>

-- Sent from my iPhone

debasishg commented 2 years ago

@hughsimpson Left a comment .. a version needs a change. Otherwise it looks fine. Will merge once you change the version.

hughsimpson commented 2 years ago

Pushed. Thanks!

debasishg commented 2 years ago

somehow I get a green test locally on my machine but it fails in the CI ..

hughsimpson commented 2 years ago

Sorry, I'm on holiday now. I really want to help fix this, but I'm not sure I'll be very available. I won't take it amiss if you revert, because I can't promise I'll have the time to fix this in the next week or two, but I'll try.

debasishg commented 2 years ago

No problem .. I will take a look.