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 most of the remaining math filters #28

Merged
merged 2 commits into from
Apr 2, 2016

Conversation

teburd
Copy link
Contributor

@teburd teburd commented Mar 29, 2016

  • times
  • divided_by
  • ceil
  • floor
  • round

@johannhof
Copy link
Contributor

Hey, thanks for contributing! Your PR is looking good, just two minor things:

  • you'll have to rebase on master, sorry for that :)
  • I'd love if you could add assertions for the error returning paths as well, doesn't have to be fancy (e.g. call "ceil" with a string and check it returns an error)

Thank you!

* times
* divided_by
* ceil
* floor
* round
@teburd
Copy link
Contributor Author

teburd commented Mar 31, 2016

I spent few thinking about a way to make a macro for testing error returns without adding a bunch of fluff, might take me awhile. This was rebased otherwise though with the success cases tested so far.

@johannhof
Copy link
Contributor

Can't you just do

assert!(ceil(Bool(true), &[]).is_err());

? :)

@teburd
Copy link
Contributor Author

teburd commented Apr 1, 2016

Right, I was for whatever reason thinking testing the error type and string would also be good, but I guess just showing its an error scenario would be good enough

@johannhof
Copy link
Contributor

Great job, thank you! 👍

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