-
Notifications
You must be signed in to change notification settings - Fork 213
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
Decode: don't panic when date time is missing timezone #614
Conversation
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.
Thank you for submitting a patch! We purposefully do not use regexps in this library because they are redundant (we're already writing a validating state machine), slower, and incurs memory allocation. We need a different solution for this problem.
Got it, will work on a different solution |
@pelletier Made changes to not use regexp |
Thanks for the patch! Removed a debug statement and added a test to double check the reported issue was fixed. |
Thanks @pelletier, will be great if you could add the hactoberfest-accepted label just in case hactoberfest topic is not there for repo |
@RiyaJohn added, but there is the |
Issue: #596
Explanation of what this pull request does.
Fixes the issue of throwing unexpected panic
Returns more meaningful error for the user
More detailed description of the decisions being made and the reasons why (if the patch is non-trivial).