-
Notifications
You must be signed in to change notification settings - Fork 9.1k
- adds a char format description #3185
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
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.
I have an impulse to note that it's more reliable for validation to use this alongside minLength
and maxLength
, but you can say similar things about all uses of format
so it's probably best to not complicate things.
@handrews people could use both:
This way they'd have validation from both tooling that doesn't understand the format and from the tooling that does. Do you think we should add a note? |
@baywet no, it's probably best not to as we'd then want to add notes to all of the formats about how to compensate for format support being a dumpster fire in general. People use |
makes sense. I do think there should be a place for "things that are not normatize but that people should consider". Kind of a collection of best practices if that makes sense. |
@baywet in the absence of a set of technical notes or authorative blog posts, possibly https://github.com/OAI/Documentation/blob/main/best-practices.md ? |
Thanks. This is a bit buried, does it get published anywhere? Should we move it to the GitHub pages branch? |
A separate repo was considered preferable, I think partly due to different licensing (CC versus Apache-2.0). It is published at https://oai.github.io/Documentation and linked to from the website, but there are plans afoot to create a https://learn.openapis.org subdomain. |
Thanks, let log an issue to document that "best practice" should this format get merged. |
89b9edd
to
24146b7
Compare
@darrelmiller I believe this one is good to be merged as we've been through all the review comments. |
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
related #3167