Skip to content
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

feat: add period format #1383

Closed
wants to merge 1 commit into from
Closed

Conversation

pwmcintyre
Copy link

@pwmcintyre pwmcintyre commented Feb 27, 2023

Similar to #718 — this proposal completes the set by adding the period format — as specific in RFC3339 Appendix A

Snippet:

Periods:

   period-explicit   = iso-date-time "/" iso-date-time
   period-start      = iso-date-time "/" duration
   period-end        = duration "/" iso-date-time

   period            = period-explicit / period-start / period-end

https://datatracker.ietf.org/doc/html/rfc3339#appendix-A

@gregsdennis
Copy link
Member

Thanks, but at this time, I don't think we're up for adding onto format. It has a history of poor support and interoperability. Even the tests for validating format are optional.

However, as an annotation keyword (its default mode), you can just put format: period, and most validators will just allow the value. You'll need to actually do the validation yourself in app code.

Beyond that, I think your best bet is to work with the implementation you're using to see what support they have for custom formats.

@awwright
Copy link
Member

As a well-defined and useful standard, I think this makes sense to be in core. But like @gregsdennis mentioned, there has been a bit of a change happening in how we approach "format" keywords and extensibility in general, so I filed #1391 to address that aspect.

Is "period" discussed in any other repo? If so, I think that issue plus 1391 will suffice and we can close this.

@awwright awwright closed this Mar 27, 2023
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.

3 participants