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.

Sign up for GitHub

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

Jump to bottom

Add newtype, TH functions for deriving instances from backend compat #1179

Merged
merged 4 commits into from
Mar 9, 2021
Merged

Add newtype, TH functions for deriving instances from backend compat #1179

merged 4 commits into from
Mar 9, 2021

Conversation

ivanbakel
Copy link
Contributor

@ivanbakel ivanbakel commented Dec 21, 2020 •
edited
Loading

If a backend is compatible with another, it has a lot of obvious instances for various Persistent classes in terms of the backend it is compatible with - RawSqlite in the codebase is one example.

This adds a newtype, Compatible, which has these compatible instances declared on it. Compatible is designed to be used with DerivingVia to automatically yield these compatible instances when given a witness of backend compatibility.

To help use Compatible and write all the standalone derivations necessary to get a fully-featured compatible backend, this also adds some TH invocations to generate the standalone derivations for a given pair of compatible backend types.

RawSqlite is rewritten to dogfood on thes invocations, to prove their effectiveness and simplify the code somewhat.

Before submitting your PR, check that you've:

After submitting your PR:

  • Update the Changelog.md file with a link to your PR
  • Bumped the version number if there isn't an (unreleased) on the Changelog
  • Check that CI passes (or if it fails, for reasons unrelated to your change, like CI timeouts)

parsonsmatt
parsonsmatt approved these changes Jan 6, 2021
View reviewed changes
Copy link
Collaborator

@parsonsmatt parsonsmatt left a comment

Choose a reason for hiding this comment

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

This is awesome! Just one style question.

I'd like to batch this up with 2.12, but I can also release as 2.12.1 if you can't get to it soon.

Thanks!

persistent/Database/Persist/Compatible.hs Outdated
( Compatible(..)
, makeCompatibleInstances
, makeCompatibleKeyInstances
) where
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we stick with 4 space indentation here? I know the codebase isn't consistent, but I want to get it moving in that direction. Thanks!

Copy link
Contributor Author

@ivanbakel ivanbakel Jan 7, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Sure! Is it just the first indent that's 4-space, and should I convert the other files to it as well? I couldn't find a clear example in the codebase.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't worry about converting anything old, but anything new should be 4 space. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alright, I hope that's done correctly.

@ivanbakel
Add newtype, TH functions for deriving instances from backend compat
b553ba2 
If a backend is compatible with another, it has a lot of obvious
instances for various Persistent classes in terms of the backend it is
compatible with - RawSqlite in the codebase is one example.

This adds a newtype, Compatible, which has these compatible instances
declared on it. Compatible is designed to be used with DerivingVia to
automatically yield these compatible instances when given a witness of
backend compatibility.

To help use Compatible and write all the standalone derivations
necessary to get a fully-featured compatible backend, this also adds
some TH invocations to generate the standalone derivations for a given
pair of compatible backend types.

RawSqlite is rewritten to dogfood on thes invocations, to prove their
effectiveness and simplify the code somewhat.
@ivanbakel
Add Changelong, since declarations
c6905f5
@ivanbakel
Fix indentation in Compatible.* modules
12acb1a
@ivanbakel
Copy link
Contributor Author

This build failure is interesting - the new code doesn't really make sense to have on old GHC versions, since if you don't have DerivingVia, you're not going to need the newtype. However, the RawSqlite code still needs its instances - so I could keep the current instances as a conditional compilation for if the GHC version is low enough.

@ivanbakel
Make Database.Persist.Compatible backwards-compatible
ca54341 
The Compatible modules are now elided for old GHC versions.

The RawSQLite compatibility instances are now polyfilled manually for
old enough versions of `base`.
@ivanbakel
Copy link
Contributor Author

Alright, sorry for the delay in fixing this, but it should now work across all the supported GHC versions. The instances get polyfilled for older GHCs which don't have DerivingVia and the new Compatible modules aren't exposed for old-enough GHC versions.

@parsonsmatt
Copy link
Collaborator

Sorry took me so long! LGTM, thanks so much 😄

@parsonsmatt parsonsmatt merged commit 929b8e8 into yesodweb:master Mar 9, 2021
vapaj pushed a commit to KSF-Media/persistent that referenced this pull request Mar 10, 2021
@ivanbakel
Add newtype, TH functions for deriving instances from backend compat (y…
7b446fe 
…esodweb#1179)

* Add newtype, TH functions for deriving instances from backend compat

If a backend is compatible with another, it has a lot of obvious
instances for various Persistent classes in terms of the backend it is
compatible with - RawSqlite in the codebase is one example.

This adds a newtype, Compatible, which has these compatible instances
declared on it. Compatible is designed to be used with DerivingVia to
automatically yield these compatible instances when given a witness of
backend compatibility.

To help use Compatible and write all the standalone derivations
necessary to get a fully-featured compatible backend, this also adds
some TH invocations to generate the standalone derivations for a given
pair of compatible backend types.

RawSqlite is rewritten to dogfood on thes invocations, to prove their
effectiveness and simplify the code somewhat.

* Add Changelong, since declarations

* Fix indentation in Compatible.* modules

* Make Database.Persist.Compatible backwards-compatible

The Compatible modules are now elided for old GHC versions.

The RawSQLite compatibility instances are now polyfilled manually for
old enough versions of `base`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@parsonsmatt parsonsmatt parsonsmatt approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ivanbakel @parsonsmatt