Skip to content

Allow for time-numoffset with and without colon separator #838

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

Open
wants to merge 1 commit into
base: draft-06-support
Choose a base branch
from

Conversation

matason
Copy link

@matason matason commented Aug 13, 2025

closes #837

@DannyvdSluijs
Copy link
Collaborator

@matason Thanks for the contribution. I've done some checking and the test case that is removed here has been part of the library for the past 12 years. I noticed that in the Draft06 branch the RFC3339 class was altered so I believe the regression occurs somewhere in that class and the fix should be applied there as well.

Since the test case has been part of the codebase for the past 12 years there is a highly likely chance it is being used somewhere.

Would you mind taking another look and see if you can update to PR to fix the issue in the RFC3339 class?

@matason
Copy link
Author

matason commented Aug 22, 2025

@DannyvdSluijs that’s a really good point, that that has been in there for so long is a good indicator that removing it is not the right thing to do. I’ll absolutely have a look at the class and see what I can figure out.

Thanks for the kind reply.

@matason matason changed the title Remove invalid datetime in test Allow for time-numoffset with and without colon separator Aug 23, 2025
@matason
Copy link
Author

matason commented Aug 23, 2025

Hi there,

I've put back the 2000-05-01T12:12:12+0100 test case in the data provider and made a small change to the regex to allow for an offset with and without the colon separator.

I'm still wondering whether an offset without a colon is valid - https://datatracker.ietf.org/doc/html/rfc3339#section-5.6 has time-numoffset = ("+" / "-") time-hour ":" time-minute but perhaps I am missing something.

On the current draft-06-support branch, this commit reduces the test failures from 15 to 13.

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