-
Notifications
You must be signed in to change notification settings - Fork 110
Add support for RFC 2822 in DateTime #285
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 support for RFC 2822 in DateTime #285
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.
Code looks great and I'm willing to accept it as is - but I do think the naming needs a change. The existing fromIsoDate
is really just another "rfc" date spec, which is unfortunate. It is named that way to mirror the Roblox DateTime datatype which does not have anything other than rfc 3339 (which they call "iso")..
I'm open to suggestions here.
That's fair. I can make the name more verbose, like fromRfc2822Date, and add fromRfc3339Date for consistency, while also keeping fromIsoDate to match the Roblox datatype. |
I like that. The |
Great! I just committed the changes |
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.
LGTM 👍 the CI failing is just new clippy lints.. I'll get those fixed
This adds support for RFC 2822 to the DateTime library so dates in the RFC 2822 format can be parsed e.g. date headers when making requests with the Net library.
I propose a function such as fromDate is added as well, which will attempt to parse both an RFC 3339 date and RFC 2822 date, returning whichever does not fail. Erroring if neither a RFC 3339 date or RFC 2822 can't be returned. I have omitted this from the PR unless maintainers approve.
At the moment the following is the best alternative, which is why I propose this is implemented.