Derive Lift instances wherever possible#252
Merged
snoyberg merged 1 commit intoyesodweb:masterfrom Jun 18, 2020
RyanGlScott:master
Merged
Derive Lift instances wherever possible#252snoyberg merged 1 commit intoyesodweb:masterfrom RyanGlScott:master
snoyberg merged 1 commit intoyesodweb:masterfrom
RyanGlScott:master
Conversation
Many of `shakespeare`'s `Lift` instances are equivalent to what would be generate by `DeriveLift`, so this patch switches them over to use `deriving`. For those instances that do not easily admit derived instances, I made sure that `liftTyped` was defined to avoid warnings when compiling with `template-haskell-2.16.*` (GHC 8.10). One example of such an instance is the `Lift ShakespeareSettings` instance, which relies on lifting `Exp`s and `Name`s from `template-haskell`. Since there is a canonical `Lift Name` orphan instance defined in the widely used, lightweight `th-lift` package, I decided to just add a dependency on that. This allows making the implementation of the `Lift ShakespeareSettings` instance slightly cleaner.
Contributor
Author
|
There were two instances that could conceivably be derived, but I wasn't bold enough to do so:
These don't necessary need to hold up this PR, but I thought it worth noting as potential improvements in the future. |
snoyberg
reviewed
Jun 18, 2020
Member
snoyberg
left a comment
There was a problem hiding this comment.
Thanks! I'll hold off on the additional dependency for the moment, but I'm not strongly opposed to including it in the future
snoyberg
added a commit
that referenced
this pull request
Jun 18, 2020
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Many of
shakespeare'sLiftinstances are equivalent to what would be generated byDeriveLift, so this patch switches them over to usederiving. For those instances that do not easily admit derived instances, I made sure thatliftTypedwas defined to avoid warnings when compiling withtemplate-haskell-2.16.*(GHC 8.10). One example of such an instance is theLift ShakespeareSettingsinstance, which relies on liftingExps andNames fromtemplate-haskell. Since there is a canonicalLift Nameorphan instance defined in the widely used, lightweightth-liftpackage, I decided to just add a dependency on that. This allows making the implementation of theLift ShakespeareSettingsinstance slightly cleaner.