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

CE2 Resource: factor out duplication between versions, backport type safety improvements from CE3 #1215

Merged
merged 9 commits into from
Sep 25, 2020

Conversation

bplommer
Copy link
Contributor

No description provided.

@vasilmkd
Copy link
Member

Love this change. I just have a slight issue with the name of ResourceCompat. Personally, I'd go for a BaseResource or ResourceLike as it is more descriptive (again, to me). But feel free to ignore. This is more of a general question, is there a precedent for this kind of base class that works across versions?

@bplommer
Copy link
Contributor Author

bplommer commented Sep 20, 2020

Personally, I'd go for a BaseResource or ResourceLike as it is more descriptive (again, to me).

I have no view on this so am happy to change it unless others object 🙂

This is more of a general question, is there a precedent for this kind of base class that works across versions?

Not sure - I only remember seeing it done with different implementations for the same API, rather than different APIs for the same implementation.

@bplommer bplommer marked this pull request as draft September 20, 2020 16:48
@bplommer
Copy link
Contributor Author

Converting to draft because mima isn't happy with InvariantResource#invariant for some reason

@bplommer
Copy link
Contributor Author

I had to add a dummy implementation of invariant to Resource, as mima didn't like me adding an abstract method. Alternatively I could just move the real implementation from InvariantResource to Resource, but that would require asInstanceOf - thoughts?

@bplommer bplommer marked this pull request as ready for review September 20, 2020 17:15
@djspiewak
Copy link
Member

Resource is sealed (and all subtypes are non-abstract), so it's safe to add a mima exception for that particular warning.

@djspiewak djspiewak merged commit 5003d04 into typelevel:series/2.x Sep 25, 2020
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