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

Support nested records in deriveEsqueletoRecord #324

Merged
merged 8 commits into from
Aug 1, 2022

Conversation

9999years
Copy link
Contributor

@9999years 9999years commented Jul 7, 2022

Before submitting your PR, check that you've:

After submitting your PR:

  • Update the Changelog.md file with a link to your PR.
  • Check that CI passes (or if it fails, for reasons unrelated to your change, like CI timeouts).

@9999years 9999years marked this pull request as ready for review July 7, 2022 19:42
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 looks great! just needs a few tests written to ensure it works

Comment on lines 457 to 459
case filteredInstances of
sqlType : _ -> pure $ Just sqlType
_ -> pure Nothing
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
case filteredInstances of
sqlType : _ -> pure $ Just sqlType
_ -> pure Nothing
pure $ listToMaybe filteredInstances

-- Filter `instances` to the instances which match `originalType`.
filteredInstances :: [Type]
filteredInstances =
flip mapMaybe instances
Copy link
Collaborator

Choose a reason for hiding this comment

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

hm, listToMaybe . mapMaybe f is also find (isJust . f) - may be easier or more concise to write like that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think so, because find only returns the element but I also need the mapMaybe part to transform and extract the sql type.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh, good point!

@parsonsmatt parsonsmatt merged commit ff0fa54 into bitemyapp:master Aug 1, 2022
@parsonsmatt
Copy link
Collaborator

released as 3.5.6.1

@9999years 9999years deleted the rbt/dux-531-nested-records branch August 1, 2022 14:53
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.

None yet

2 participants