-
Notifications
You must be signed in to change notification settings - Fork 9.1k
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
Various minor editorial improvements (3.0.4) #3861
Conversation
Guide readers to supplemental documentation, examples, related specificatioins, and extension registries. These sites answer many questions that otherwise get raised as GitHub issues.
Co-authored-by: Ralf Handl <ralf.handl@sap.com>
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.
Really minor nitpicks, I'm happy to re-review and approve after updates
versions/3.0.4.md
Outdated
@@ -203,7 +213,7 @@ Note that the encoding indicated by `byte`, which inflates the size of data in o | |||
|
|||
### <a name="richText"></a>Rich Text Formatting | |||
Throughout the specification `description` fields are noted as supporting CommonMark markdown formatting. | |||
Where OpenAPI tooling renders rich text it MUST support, at a minimum, markdown syntax as described by [CommonMark 0.27](https://spec.commonmark.org/0.27/). Tooling MAY choose to ignore some CommonMark features to address security concerns. | |||
Where OpenAPI tooling renders rich text it MUST support, at a minimum, markdown syntax as described by [CommonMark 0.27](https://spec.commonmark.org/0.27/). Tooling MAY choose to implement extensions on top of CommonMark 0.27, and MAY choose to ignore some CommonMark or extension features to address security concerns. |
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'm conflicted about expressly encouraging different tools to support different formats (conflicted because I work for a tools vendor that supports ridiculously good extensions that breaks other tooling if used!). The extensions were sort of already implied with "at a minimum", but this is a step further and I'm not sure it's helpful to users if they are expecting their API descriptions to be interoperable.
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.
@lornajane I only want to clarify the original intention. I don't at all strongly about this, it's just one more issue to resolve. If you and the TSC want to resolve the issue (#1867) without action I'm happy to take this commit out.
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.
@lornajane I updated this to warn about interoperability problems and offer guidance to description authors to consider the impact on users of tools that don't support extensions. Please let me know if you think this is a good balance. I'm also still happy to strike this change if it seems problematic.
Co-authored-by: Lorna Jane Mitchell <github@lornajane.net>
Co-authored-by: Lorna Jane Mitchell <github@lornajane.net>
Co-authored-by: Lorna Jane Mitchell <github@lornajane.net>
Thanks to @lornajane for the review feedback.
Also clarified |
When we mention YAML's "Failsafe schema" we give it a lower-case "schem", as the YAML documentatio does. We also prefix it with "YAML". However, we capitalize "Schema" in "JSON Schema ruleset", which (given how much JSON Schema is used in the OAS) is a jarring overlap with "JSON Schema". This change aligns "YAML JSON schema ruleset" with "YAML Failsafe ruleset" and explicitly calls out that it is unrelated to JSON Schema.
I've added one more clarification that's been bugging me for years since this is stil open: Clarify confusing use of YAML "JSON Schema" When we mention YAML's "Failsafe schema" we give it a lower-case"schema", as the YAML documentatio does. We also prefix it with "YAML". However, we capitalize "Schema" in "JSON Schema ruleset", which (given how much JSON Schema is used in the OAS) is a jarring overlap with "JSON Schema". This change aligns "YAML JSON schema ruleset" with "YAML Failsafe ruleset" and explicitly calls out that it is unrelated to JSON Schema. |
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.
The updated wording strikes a decent balance IMO, thanks @handrews !
Fixes: