-
Notifications
You must be signed in to change notification settings - Fork 5
added new error handling to date and datetime #133
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
base: main
Are you sure you want to change the base?
added new error handling to date and datetime #133
Conversation
chawes13
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you bump the version number as well?
bumped the minor version number |
chawes13
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the radio silence 📻 🫣. A few comments regarding some design choices, but overall super close!
nicoledow
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the wait on this. I thought there were changes requested, but I was mistaken!
This looks and works great, just left a couple minor questions.
| @@ -1,3 +1,3 @@ | |||
| module Decanter | |||
| VERSION = '5.0.0'.freeze | |||
| VERSION = '5.1.0'.freeze | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The latest from main may need to be pulled in here - we're at 5.1.0 now so this can be bumped up to 5.2.0
|
|
||
| parser do |val, options| | ||
| next if (val.nil? || val === '') | ||
| raise Decanter::ParseError, 'Expects a single value' if val.is_a? Array |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this line can be removed since this parser inherits from ValueParser, which raises an error if the value is an array
Items Addressed
feature request
Decanter::ValueFormatError '(invalid (Date/DateTime) value for format'