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

Reject negative scalars in for-duration calculations #60

Merged
merged 4 commits into from
Apr 12, 2016

Conversation

craiglittle
Copy link
Collaborator

Negative scalars are not something we want to explicitly support at this time, so we'll raise a helpful error message instead of randomly blowing up when someone tries to do it.

This also includes a bit of work to streamline and standardize exceptions.

Fixes #59.

@alex-stone @joshlam @kpandya91 @kbrainwave

/cc @mwean

@craiglittle
Copy link
Collaborator Author

After giving it more thought, I feel comfortable supporting the zero case whereby the first active moment in the chosen direction will be returned.

I'll modify this PR to restrict the validation to rejecting negative values and follow up with another to support "zero" calculations.

Stay tuned.

@craiglittle craiglittle force-pushed the craig/zero-for-duration branch 2 times, most recently from 12f63a7 to 8dc3737 Compare April 11, 2016 22:28
@craiglittle craiglittle changed the title Validate ForDuration scalar value Reject negative scalars in for-duration calculations Apr 11, 2016
@craiglittle craiglittle force-pushed the craig/zero-for-duration branch from 8dc3737 to 880888a Compare April 11, 2016 22:32
@craiglittle
Copy link
Collaborator Author

💇 I've modified this PR to focus on the new scalar validation (non-negative) and error streamlining. I'll be opening another to support zero-scalar calculations.

Negative scalars are not something we want to explicitly support at
this time, so we'll raise a helpful error message instead of randomly
blowing up when someone tries to do it.
If we're primarily testing for the presence or absence of an exception
being raised, let's do that consistently.
* Downcase first word (this is consistent with core Ruby)
* Remove period at the end (also consistent with core Ruby)
* Cut out unnecessary words
The generic `RuntimeError` was being raised when using `biz` before
it is configured. Since we have a more descriptive configuration error,
we should use it.
@craiglittle craiglittle force-pushed the craig/zero-for-duration branch from 880888a to 99a971f Compare April 12, 2016 01:19
@alex-stone
Copy link

👍

@craiglittle craiglittle merged commit 7c43f3b into master Apr 12, 2016
@craiglittle craiglittle deleted the craig/zero-for-duration branch April 12, 2016 17:09
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