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

Update scala 3 version, drop support for scala 2 #527

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

hughsimpson
Copy link
Contributor

@hughsimpson hughsimpson commented Aug 1, 2024

Pull Request Checklist

  • I agree to licence my contributions under the MIT licence
  • I have added copyright headers to new files -- No new files
  • I have added tests for any changed functionality -- No added functionality

Removes scala 2, updates scala 3 to 3.4.2, adds -experimental compiler flag to negate need for bug/workaround

@hughsimpson hughsimpson changed the title Fix mocking classes with parameterised constructor args Fix mocking classes with parameterised constructor args in scala 3 Aug 1, 2024
@hughsimpson hughsimpson changed the title Fix mocking classes with parameterised constructor args in scala 3 Fix mocking classes with parameterised constructor args in scala 3 / Update scala 3 version Aug 1, 2024
@barkhorn barkhorn added this to the v6.1.0 milestone Aug 2, 2024
@hughsimpson
Copy link
Contributor Author

Hadn't updated the scala 3 version in the GitHub workflow so that test failed. Pushed an update there

@hughsimpson hughsimpson changed the title Fix mocking classes with parameterised constructor args in scala 3 / Update scala 3 version Update scala 3 version Aug 5, 2024
@hughsimpson
Copy link
Contributor Author

So anyway this whole pr was way off base and I do apologise for that. I guess I figured it might still be worth leaving open on the off-chance, and because it had already been tagged (presumably because of my initial false claim about the downstream impact 😬). I was tempted to close it or mark as draft but... idk. Potentially scala 3.4 warrants discussing even though I originally introduced the idea for bad reasons

@barkhorn
Copy link
Collaborator

Hey, i don't know about that. It will need a review for sure if there's a potential impact. But we should be able to follow new Scala versions in principle unless there's some breaking changes. After all, if someone wants a ScalaMock for an older versions, Sonatype (to my knowledge) doesn't purge versions so there's always a fallback

@martijnhoekstra
Copy link
Contributor

As a library, I'd suggest staying on LTS versions, as not to force downstreams to update

@goshacodes
Copy link
Contributor

goshacodes commented Sep 14, 2024

I think there is no problem of bumping version up to latest 3.4 or even 3.5.0 if we can get rid of annotating everything as @experimental, this is crucial, essentially for library users. Scala 3 offers backward compatibility for all versions.

Anyway we are exploiting a bug in the compiler to make it work.

If we decide to go experimental way, I would also consider using TupledFunction to make scalamock simpler

@goshacodes
Copy link
Contributor

goshacodes commented Sep 14, 2024

Also, in my opinion, we should drop support for Scala 2.12 and Scala 2.13, since library (scala 2) is stable enough and nobody would fix anything for scala 2 anyway. This would unlock futher library development.
I have already made the migration as smooth as possible and now we should really move on or the library becomes dead soon

@hughsimpson
Copy link
Contributor Author

@goshacodes I love that energy. Happy to drop scala 2 support in this pr. Will look into your suggestions on this after I've addressed the ones on the other two.

@barkhorn
Copy link
Collaborator

Agree on Scala 2.x support, we have a maintenance-5.x branch that could be used to patch bugs there, but it's been pretty hard to work out some of the macro-related problems in the past, I certainly tried :D

@goshacodes
Copy link
Contributor

Let's proceed then. @hughsimpson please drop scala 2 in this PR

@hughsimpson hughsimpson changed the title Update scala 3 version Update scala 3 version, drop support for scala 2 Sep 21, 2024
@hughsimpson
Copy link
Contributor Author

@goshacodes Dropped scala 2, haven't looked into TupledFunction yet - probably won't have the bandwidth for a few days now - but that can probably come later anyway?

@goshacodes
Copy link
Contributor

@goshacodes Dropped scala 2, haven't looked into TupledFunction yet - probably won't have the bandwidth for a few days now - but that can probably come later anyway?

At first lets rewrite everything to scala 3 with scalafix (or similar utility, not sure which one does it)

With tupled functions research is needed anyway, If you want to take it, it will be great

@goshacodes
Copy link
Contributor

goshacodes commented Sep 21, 2024

@barkhorn Could you please close all scala-2 related issues with wont-fix to cleanup everything, after this MR is merged

Also this is not related, but If you know, could you please tell us, what does proxy package meant for?
Not found anything related in the docs or in the issues

@goshacodes
Copy link
Contributor

@hughsimpson opened an issue about TupledFunction #533

@hughsimpson
Copy link
Contributor Author

Not sure if I'll get to it or not... Once the next release is cut with the bugfix prs I'll probably be snowed under trying to finalise some chunky migrations, so will probably vanish off the reservation for a while tbqh. But if it's still waiting for me when that work is done I'll be diving straight in

@barkhorn
Copy link
Collaborator

@barkhorn Could you please close all scala-2 related issues with wont-fix to cleanup everything, after this MR is merged

Also this is not related, but If you know, could you please tell us, what does proxy package meant for? Not found anything related in the docs or in the issues

The proxy package is using Java reflection, specifically proxies, to dynamically instantiate a mock object. It's very limited compared to macro mocking but the implementation is very concise (https://github.com/paulbutcher/ScalaMock/blob/master/shared/src/main/scala-2/org/scalamock/proxy/ProxyMockFactory.scala), and there may be corner cases where it was/is used to work around some of the macro problems with the Scala 2 macro implementation.
https://docs.oracle.com/javase/8/docs/api/java/lang/reflect/Proxy.html

With a Scala 3 focus, there is probably not a great rationale anymore for this package

@hughsimpson
Copy link
Contributor Author

I don't mind ripping out a bit more whilst I'm in here. @barkhorn @goshacodes you think this pr should go further? Or merge as it stands and perform the rest of the actions in follow-up?

@goshacodes
Copy link
Contributor

I think we should merge as it is now and handle it later. Just asked)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In progress
Development

Successfully merging this pull request may close these issues.

4 participants