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

add symbol, symbols #298

Merged
merged 2 commits into from
Aug 15, 2022
Merged

add symbol, symbols #298

merged 2 commits into from
Aug 15, 2022

Conversation

ocramz
Copy link
Contributor

@ocramz ocramz commented Aug 14, 2022

Add smart constructors for Symbol , with the long-term goal of deprecating the IsList instance.

I should note in passing that the GHC DEPRECATE pragma doesn't work with class instances, which leaves the fix up to a judicious versioning schedule

Addresses (partly) #297

@snowleopard
Copy link
Owner

snowleopard commented Aug 14, 2022

Thanks! The change looks good to me. I left a little suggestion.

I think we can drop the IsList instance altogether if it's problematic.

I should note in passing that the GHC DEPRECATE pragma doesn't work with class instances, which leaves the fix up to a judicious versioning schedule

I didn't quite get what you meant here. Could you elaborate?

@ocramz
Copy link
Contributor Author

ocramz commented Aug 15, 2022

judicious versioning schedule

I didn't quite get what you meant here. Could you elaborate?

Just that removing class instances would need bumping a major version number, and possibly warning users beforehand.

@ocramz
Copy link
Contributor Author

ocramz commented Aug 15, 2022

Also, I removed mention of the Dioid (Label a) instance because I couldn't find it.

@snowleopard
Copy link
Owner

Just that removing class instances would need bumping a major version number, and possibly warning users beforehand.

Ah, got it now. Agreed.

@snowleopard snowleopard merged commit 601caa8 into snowleopard:master Aug 15, 2022
@snowleopard
Copy link
Owner

Thanks, merged!

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