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

added coalesce/coalesceDefault functions #71

Merged
merged 2 commits into from
Jul 12, 2014

Conversation

mitchellwrosen
Copy link
Contributor

I'm not sure if coalesceDefault really belongs, but I think it might be necessary to avoid having to write partial functions (e.g. let Just x = unValue val).

-- | Like @coalesce@, but takes a non-nullable expression
-- placed at the end of the expression list, which guarantees
-- a non-NULL result.
coalesceDefault :: PersistField a => expr (Value a) -> [expr (Value (Maybe a))] -> expr (Value a)
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't the default value be the second argument since it's going to be the at the end?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I was thinking sort of like Data.Maybe.fromMaybe argument order, but you're right

@meteficha
Copy link
Member

Your code looks pretty solid! I just have the small comment I made above. After that, I'll merge and release a new version.

@meteficha
Copy link
Member

By the way, sorry for the noise of the duplicated messages, I'm using a not-so-stable code review app :).

@mitchellwrosen
Copy link
Contributor Author

Cool, done. Thanks!

meteficha added a commit that referenced this pull request Jul 12, 2014
added coalesce/coalesceDefault functions
@meteficha meteficha merged commit dc285e4 into prowdsponsor:master Jul 12, 2014
@meteficha
Copy link
Member

Thanks! Merge & released as esqueleto-1.4.3.

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