Implement Type Literal based field definitions by danbroooks · Pull Request #1343 · yesodweb/persistent

danbroooks · 2021-11-28T20:30:32Z

Implements #562

Before submitting your PR, check that you've:

Documented new APIs with Haddock markup
Added @since declarations to the Haddock
Ran stylish-haskell on any changed files.
Adhered to the code style (see the .editorconfig file for details)

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)

danbroooks · 2021-11-28T20:33:40Z

persistent/Database/Persist/Types.hs

       , FieldCascade(..)
       , FieldDef(..)
       , FieldType(..)
+       , FieldTypeLit(..)


Given the comment at the top here:

persistent/persistent/Database/Persist/Types.hs

Lines 48 to 50 in 77545dc

-- this module is a bit of a kitchen sink of types and concepts. the guts of

-- persistent, just strewn across the table. in 2.13 let's get this cleaned up

-- and a bit more tidy.

I wasn't sure if this was the right place for this import, but seemed like it was where everything else was re-exported (I need to use this type in Persist/TH.hs I think...)

Going forward should I import this from Database.Persist.Types.Base ? Or is re-exporting it here correct for the time being?

I'd rather not expose data constructors to the end-user in a non Internal module, since any change to those constructors necessitates a breaking change to the API version.

In this case, I'd be fine exporting it from Database.Persist.Quasi.Internal, since that's where it is used.

danbroooks · 2021-11-28T20:35:35Z

persistent/test/Database/Persist/QuasiSpec.hs

+                        [ Token "one"
+                        , Token "Finite 1"
+                        ]
+                    )


This was an initial test I did as an exploratory thing to verify the parser was even parsing numbers at all to begin with (which it was), this test could potentially be removed now. Though it may also be worthwhile leaving in.

Yeah! It's good to have around as a regression. Thanks for the test!

parsonsmatt

Thanks! This is great 😂

OK, so, since this is modifying a constructor on an exposed type (FieldType), then it's a major version bump. So it'll go in with persistent-2.14.

parsonsmatt · 2022-01-27T00:41:57Z

persistent/Database/Persist/Quasi/Internal.hs

+                <|> parseParenEnclosed x xs
+                <|> parseList x xs
+                <|> parseNumericLit x xs
+                <|> parseTextLit x xs
+                <|> parseTypeCon x xs


Nice - this is great. Moving away from ad-hoc text munging and instead using more idiomatic Haskell classes.

parsonsmatt · 2022-01-27T00:43:54Z

persistent/Database/Persist/Types.hs

       , FieldCascade(..)
       , FieldDef(..)
       , FieldType(..)
+       , FieldTypeLit(..)


I'd rather not expose data constructors to the end-user in a non Internal module, since any change to those constructors necessitates a breaking change to the API version.

parsonsmatt · 2022-01-27T00:44:37Z

persistent/Database/Persist/Types.hs

       , FieldCascade(..)
       , FieldDef(..)
       , FieldType(..)
+       , FieldTypeLit(..)


In this case, I'd be fine exporting it from Database.Persist.Quasi.Internal, since that's where it is used.

parsonsmatt · 2022-01-27T00:45:01Z

persistent/test/Database/Persist/QuasiSpec.hs

+                        [ Token "one"
+                        , Token "Finite 1"
+                        ]
+                    )


Yeah! It's good to have around as a regression. Thanks for the test!

parsonsmatt · 2022-01-27T00:46:04Z

persistent/test/Database/Persist/TH/TypeLitFieldDefsSpec.hs

+|]
+
+spec :: Spec
+spec = describe "TypeLitFieldDefs" $ do


awesome specs

parsonsmatt · 2022-01-27T00:49:32Z

Oh - this also needs a bump/changelog entry for persistent-test. And it may be a good idea to start putting a lower bound on persistent-test for the test-suites of the other packages.

parsonsmatt · 2022-04-12T02:33:22Z

hmm, we have a test failure now. would like to get this merged in before 2.14, will try and get it sorted if you can't get to it soon

thanks!

* Implement Type Literal based field definitions (#1343) * Implement typelit field definitions * Some cleanup * Sort indentation and add PR link in changelog * Move location of exposed type Co-authored-by: Matt Parsons <[email protected]> * fix tests Co-authored-by: Dan Brooks <[email protected]>

danbroooks commented Nov 28, 2021

View reviewed changes

danbroooks force-pushed the persist-entity-type-lits branch 2 times, most recently from 8cf0393 to 85e2ee8 Compare November 28, 2021 21:18

danbroooks added 2 commits November 28, 2021 21:54

Implement typelit field definitions

3a10b80

Some cleanup

8f46fd8

danbroooks force-pushed the persist-entity-type-lits branch from 85e2ee8 to 8f46fd8 Compare November 28, 2021 21:55

Sort indentation and add PR link in changelog

e0eeaa7

parsonsmatt approved these changes Jan 27, 2022

View reviewed changes

parsonsmatt added this to the 2.14 milestone Jan 27, 2022

danbroooks added 2 commits January 29, 2022 12:30

Merge branch 'master' into persist-entity-type-lits

06e93e7

Move location of exposed type

929bc97

danbroooks force-pushed the persist-entity-type-lits branch from 929ee46 to f940172 Compare January 29, 2022 14:53

Merge branch 'master' into persist-entity-type-lits

cc95228

danbroooks force-pushed the persist-entity-type-lits branch 3 times, most recently from cc95228 to 929bc97 Compare March 16, 2022 19:02

danbroooks and others added 3 commits April 3, 2022 15:14

Merge branch 'master' into persist-entity-type-lits

1ddbc3b

Merge branch 'master' into persist-entity-type-lits

b5d8cfb

Merge branch 'master' into persist-entity-type-lits

4986a51

parsonsmatt added 2 commits April 11, 2022 21:46

Merge branch 'master' into persist-entity-type-lits

18ed3e8

Merge branch 'master' into persist-entity-type-lits

dc436da

parsonsmatt changed the base branch from master to matt/finalize-type-lit-pr April 12, 2022 04:18

parsonsmatt merged commit c3886bf into yesodweb:matt/finalize-type-lit-pr Apr 12, 2022

parsonsmatt mentioned this pull request Apr 12, 2022

Finalize type lits #1388

Merged

7 tasks

danbroooks deleted the persist-entity-type-lits branch March 29, 2025 18:28

	-- this module is a bit of a kitchen sink of types and concepts. the guts of
	-- persistent, just strewn across the table. in 2.13 let's get this cleaned up
	-- and a bit more tidy.

Conversation

danbroooks commented Nov 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

parsonsmatt left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

parsonsmatt commented Jan 27, 2022

Uh oh!

parsonsmatt commented Apr 12, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

danbroooks commented Nov 28, 2021 •

edited

Loading