Skip to content

Add withXXX methods for easy IORuntime configuration #3192

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

Conversation

TimWSpence
Copy link
Member

No description provided.

@TimWSpence
Copy link
Member Author

@armanbilge how about this? Also I don't know why CI didn't run...

Copy link
Member

@armanbilge armanbilge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the long run I think IORuntimeConfig needs to no longer be a case class, because otherwise we are paying the cost of public copys and with methods.

@armanbilge armanbilge closed this Oct 5, 2022
@armanbilge armanbilge reopened this Oct 5, 2022
@TimWSpence TimWSpence closed this Oct 6, 2022
@TimWSpence TimWSpence reopened this Oct 6, 2022
@djspiewak
Copy link
Member

Is this really better than just using copy though?

@armanbilge
Copy link
Member

Yes, if we de-case-classify IORuntime. copy methods are worse for bincompat since they need to be overloaded.

@djspiewak
Copy link
Member

Right but if we keep IORuntime as a case class, then this is kind of redundant, isn't it? Definitely get why copy is a pain for bincompat.

@armanbilge
Copy link
Member

Right but if we keep IORuntime as a case class

Which is terrible for bincompat :) For example case classes generate mirrors on Scala 3 which we would have to be hand-maintained each time we change it.

Indeed, I suspect we've already broken it and our over-aggressive filters are just not telling us.

@armanbilge
Copy link
Member

armanbilge commented Oct 9, 2022

Hmm, I take that back. I guess the mirrors can evolve bincompatibly without intervention (edit: although they may throw runtime errors 🤔 ). Also I am concerned about the unapply methods, but either those are fine or are slipping through due to type erasure. In any case, we don't have any filters for IORuntimeConfig /shrug

@TimWSpence
Copy link
Member Author

Sorry to only just get to this. Where did we land with this? Shall I just close it?

@djspiewak djspiewak added this to the v3.5.0 milestone Jan 16, 2023
@djspiewak
Copy link
Member

Okay coming back to this… (finally)

I think let's close this for now. If we un-case-class-ify IORuntimeConfig, we'll revive this approach.

@djspiewak djspiewak closed this Feb 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants