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

docs: clarify bundling behavior on Schema Object #653

Merged
merged 4 commits into from
Dec 8, 2021

Conversation

smoya
Copy link
Member

@smoya smoya commented Nov 12, 2021

This PR clarifies the behavior of bundling and (de)referencing of Schema Object, introducing a clear statement and reminder that the $ref behavior does not follow JSON Schema one, therefore things like $id won't have any effect on it.

Related issue(s):

#649

@smoya smoya added the ✏️ Editorial PR is non-normative or does not influence implementation label Nov 12, 2021
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.

spec/asyncapi.md Outdated
@@ -1670,6 +1669,7 @@ The following properties are taken from the JSON Schema definition but their def
- default - The default value represents what would be assumed by the consumer of the input as the value of the schema if one is not provided. Unlike JSON Schema, the value MUST conform to the defined type for the Schema Object defined at the same level. For example, of `type` is `string`, then `default` can be `"foo"` but cannot be `1`.

Alternatively, any time a Schema Object can be used, a [Reference Object](#referenceObject) can be used in its place. This allows referencing definitions in place of defining them inline.
It is appropriate to clarify that the `$ref` keyword will follow the behavior described by [Reference Object](#referenceObject) **and not** the one in JSON Schema definition. Therefore, `$id` keyword won't modify the behavior of `$ref`.
Copy link
Member

@jonaslagoni jonaslagoni Nov 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To further clarify this problem people will need to wrap their head around what $id is and do (we don't describe any of this in the schema object), what JSON Schema is, how it relates to the default super-set of JSON Schema, and this is not relevant to users if they aren't using JSON Schema at all (hope that made sense 😅)

Not sure if we can really clarify this, on the other hand, it is kinda needed... Idk how we can solve this 🙃 😆

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I see your point. However, I would like to clarify some things first:

we don't describe any of this in the schema object

In the "Schema Object" section, we mention it is a superset of JSON Schema Draft-07, including a link to the core and validation documents. Also, we have this statement As such, any keyword available for those vocabularies is by definition available in AsyncAPI, and will work the exact same way, including but not limited to

So IMHO it is clear there is a relation and the fact we directly depend on JSON Schema Draft-07 spec.

Is it enough with just having those lines? Do you suggest we expand the information further?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But it also states further down:

Alternatively, any time a Schema Object can be used, a Reference Object can be used in its place. This allows referencing definitions in place of defining them inline.

Which is even more confusing, when they have nothing to do with each other 😆

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the reference to $id plus removed the linebreak so it stick all together. Also added a link to the JSON Schema reference docs. Hope that makes more sense now @jonaslagoni !

@smoya smoya requested a review from jonaslagoni November 15, 2021 14:13
@smoya
Copy link
Member Author

smoya commented Dec 8, 2021

cc @derberg @fmvilas you might be interested in this one! Thank you!

spec/asyncapi.md Outdated Show resolved Hide resolved
Co-authored-by: Fran Méndez <[email protected]>
@smoya smoya requested a review from fmvilas December 8, 2021 16:07
@smoya
Copy link
Member Author

smoya commented Dec 8, 2021

/rtm

@sonarqubecloud
Copy link

sonarqubecloud bot commented Dec 8, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@smoya
Copy link
Member Author

smoya commented Dec 8, 2021

/rtm

@asyncapi-bot asyncapi-bot merged commit 5f8ea79 into asyncapi:master Dec 8, 2021
@smoya smoya deleted the docs/bundlingSchemaObject branch December 8, 2021 20:48
@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 2.3.0-2022-01-release.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 2.3.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✏️ Editorial PR is non-normative or does not influence implementation ready-to-merge released on @2022-01-release released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants