| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
This moves some guidance up to the fixed fields section where it is more obvious, and explicitly designates other configurations as having undefined behavior. It also creates subsections to organize the different topics, pulls key guidance out of the examples and up into those sections, and provides clarification on the ambiguity of names and URIs. Finally, it incorporates ideas from @jdesrosiers regarding the ambiguous `mapping` syntax submitted in a prior PR, but does so in a way that meets our compatibility requirements for patch releases. For the same compatibility reasons, the MUST wording for requiring the named discriminator property in the schema was (regrettably) weakened to a "SHOULD but otherwise undefined", as we have done for other problematic ambiguities. Co-authored-by: Jason Desrosiers <jdesrosi@gmail.com>
There was a problem hiding this comment.
+1, minor nits
Sorry, something went wrong.
No thanks, you go ahead. But, I appreciate you asking and the recognition. Thanks for getting this work over the finish line. |
Sorry, something went wrong.
I found the "discriminator value" language confusing, because `discriminator` is the field name, and in most cases the value of a field is the value in the OpenAPI Description. Reviewers did not like "discriminator property value", but "discriminating value" makes it clear that this is about the value that actually causes schema selection, and not about the value of the discriminator keyword. Also removed placeholder in favor of just having the headings, as has been done in several other PRs that overlap with new section PRs for internal linking.
There was a problem hiding this comment.
Looks good
Sorry, something went wrong.
There was a problem hiding this comment.
Looks good. 👍
I commented on an empty H4. If that was not intended it should be fixed but everything else looked fine.
Sorry, something went wrong.
| #### <a name="resolvingImplicitConnections"></a>Resolving Implicit Connections | ||
|
|
There was a problem hiding this comment.
This looks like an empty H4 ... is that what was intended? It looks very odd.
Sorry, something went wrong.
There was a problem hiding this comment.
It's to keep the markdown validator from complaining about broken links. That section is being added in another PR. I have like 6 different PRs that cross-reference each other and there's no good way to do this. Of the three ways I've tried, this was the only one that @ralfhandl didn't object to.
It will all get sorted as we merge and resolve conflicts.
Sorry, something went wrong.
|
@handrews Do you want to merge now? |
Sorry, something went wrong.
Backport (3.0.4) of merge resolution from #3846
Clarify discriminator + oneOf/anyOf/allOf usage (3.2.0 port of #3846)
| Back | FazBrowse Home | New Git URL |
This is not a direct port of #3822 (although it is close), for two reasons:
I have tried to merge the improvements as best I can. There were two notable changes to the improved text:
I also revived some of @jdesrosiers 's ideas that were taken out for compatibility reasons and included a co-author credit; see the commit message below.
Once this PR is accepted, I will backport @jdesrosiers 's changes, as merged here, to 3.0.4 and include a co-author credit (unless you'd rather do it yourself, Jason).
Fixes:
Commit message:
This moves some guidance up to the fixed fields section where
it is more obvious, and explicitly designates other configurations
as having undefined behavior.
It also creates subsections to organize the different topics, pulls
key guidance out of the examples and up into those sections,
and provides clarification on the ambiguity of names and URIs.
Finally, it incorporates ideas from @jdesrosiers regarding
the ambiguous mapping syntax submitted in a prior PR, but
does so in a way that meets our compatibility requirements
for patch releases.
For the same compatibility reasons, the MUST wording for
requiring the named discriminator property in the schema
was (regrettably) weakened to a "SHOULD but otherwise undefined",
as we have done for other problematic ambiguities.
Co-authored-by: Jason Desrosiers jdesrosi@gmail.com