-
-
Notifications
You must be signed in to change notification settings - Fork 283
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
Allow spaces (and other things) as separators when parsing RFC3339 #700
Conversation
I definitely don't like this, but I also don't like this from RFC2822. It looks like tests are currently failing, as I explicitly tested for situations like this. The miri run will fail due to newer lints (it uses nightly unlike the rest of runs), so don't worry about that. With the tests updated, the change so far LGTM. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #700 +/- ##
=======================================
- Coverage 97.8% 97.6% -0.2%
=======================================
Files 81 83 +2
Lines 9378 9008 -370
=======================================
- Hits 9169 8790 -379
- Misses 209 218 +9 ☔ View full report in Codecov by Sentry. |
Edit: nope, codecov on mobile was not actually showing me the right lines. Fixing that now. |
Okay, the only failing tests is now miri, just like you said it would! This is ready for review afaict, maybe a bit heavy on the comments — if you want to trim those a bit I don't mind! Feel free to adjust the PR before merging it (if the changes otherwise look good). Note: I have not encountered any separators other than " " or "T" in the wild, so time could be stricter than RFC3339 but.. I'm not sure what the rationale would be! Your pick. |
@jhpratt Anything I can do to help this land? |
@fasterthanlime Probably the only thing would be rebasing to get miri passing. That'll save me the ~30 seconds to do it myself. Aside from that, I've been extremely busy. I do intend on reviewing this quite soon though! |
75bdad9
to
0bb11b1
Compare
Just rebased as requested! 30 seconds is 30 seconds 😎 |
492e2ee
to
d60fded
Compare
Thanks again! Sorry about the delay. |
I don't like it anymore than you probably do, but the spec says so!
(and rusqlite uses that)
cf. https://hachyderm.io/@fasterthanlime/112834567377457088