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

Add ifThenElse #37

Merged
merged 2 commits into from
Aug 11, 2017
Merged

Add ifThenElse #37

merged 2 commits into from
Aug 11, 2017

Conversation

benny-medflyt
Copy link
Contributor

This implementation is probably not perfect. And I wasn't sure about how to name things.

But I was curious about how this would be implemented, and this only took a few minutes :)

@valderman
Copy link
Owner

Do you think you could add a test case or two for this? I'd gladly merge this feature as soon as there's some automated assurance that we don't break it the next time we mess around with the code generator or backends.

@benny-medflyt
Copy link
Contributor Author

I'll try to get around to it soon.

  1. Are you satisfied with calling the function ifThenElse? (that's what it's called in opaleye)

  2. What should the haddock be? I wrote

     -- | Perform a conditional on a column
    

    But maybe you have something better..

@valderman
Copy link
Owner

I like both the name and the Haddock. Clear and to the point.

@benny-medflyt
Copy link
Contributor Author

I've rebased onto selda master, and added a test case. I've also opened #43, which if implemented would add its own tests that would also provide coverage for ifThenElse functionality

@valderman valderman merged commit 5673202 into valderman:master Aug 11, 2017
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