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

Introduce given instance #11325

Closed
wants to merge 1 commit into from
Closed

Introduce given instance #11325

wants to merge 1 commit into from

Conversation

rjolly
Copy link
Contributor

@rjolly rjolly commented Feb 5, 2021

No description provided.

@odersky
Copy link
Contributor

odersky commented Feb 5, 2021

Without an implementation this PR is not complete. Moreover it needs approval from SIP and community. So far none of that is forthcoming.

As far as I recall we did discuss instance about 18 months ago, and had a very animated discussion about it. It would be good to go back to this and find out why instance was not adopted, and how the current design might change that outcome.

@odersky odersky closed this Feb 5, 2021
@rjolly
Copy link
Contributor Author

rjolly commented Feb 5, 2021

Yes, the discussion was in #6773 . A few remarks:

  1. It was a pre-using discussion. using solved a lot of issues.
  2. instance was proposed instead of given, whereas my proposal is to complement it (as some kind of modifier).
  3. One critique was that is has a specific meaning in OOP. This argument vanishes if everybody can agree that it is about a typeclass instance (which is a subset of instance in the OOP sense anyway).
  4. Hard to google : vanishes if used in conjunction with given as proposed.

I would be happy to (try and) implement it. However, I would first like to make sure the PR will not be rejected in the end. In any case, I can understand it if you just prefer the current implementation. However, I think it would be interesting for you to revisit objections to alternative solutions, as the current one has its own flaws as already mentioned.

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.

2 participants