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 extensive descriptions and examples to the schema v2.6.0 #394

Merged
merged 29 commits into from
Jun 1, 2023
Merged

feat: add extensive descriptions and examples to the schema v2.6.0 #394

merged 29 commits into from
Jun 1, 2023

Conversation

AceTheCreator
Copy link
Member

So I decided to enhance the spec-json-schema to include additional info and examples. This will enable tools like Studio, spec-visualizer and vscode to have the ability to get more information when consuming the spec-json-schema

  • Included descriptions of the schema definitions that are missing description
  • Enhanced/Replaced existing descriptions that lack enough information
  • Added examples payload to all required definitions

cc @derberg @smoya @fmvilas

Copy link
Member

@derberg derberg left a comment

Choose a reason for hiding this comment

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

please update the PR and remove from it any changes that are not related to this PR but are a result of some weird autoformatter that you are using

in the meantime I will investigate what info do not work, and if examples play well with YAML plugin

scripts/add-new-version.js Show resolved Hide resolved
Copy link
Member

@derberg derberg left a comment

Choose a reason for hiding this comment

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

why in definitions you have example prop and then you remove it and add proper examples, why not examples from the start?

@derberg
Copy link
Member

derberg commented May 23, 2023

so I ran your script and it looks good, the info examples is added

Screenshot 2023-05-23 at 17 47 13

Copy link
Collaborator

@dalelane dalelane left a comment

Choose a reason for hiding this comment

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

looks like a lot of work that will bring quality-of-life improvements - thanks for doing it

definitions/2.6.0/parameters.json Outdated Show resolved Hide resolved
@@ -13,38 +13,48 @@
},
"properties": {
"url": {
"type": "string"
"type": "string",
"description": "A URL to the target host. This URL supports Server Variables and MAY be relative, to indicate that the host location is relative to the location where the AsyncAPI document is being served."
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't agree that we should call this a URL, but I'm being a pedant... plus that's already in https://github.com/asyncapi/spec/blob/master/spec/asyncapi.md#server-object so this isn't the PR to fix that in :-)

scripts/add-new-version.js Outdated Show resolved Hide resolved
Copy link
Member

@derberg derberg left a comment

Choose a reason for hiding this comment

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

it works well in VSCode ❤️

examples are not super nice as they are stringified
Screenshot 2023-06-01 at 14 42 10

but it is because of yaml extension and https://github.com/redhat-developer/yaml-language-server/blob/main/src/languageservice/services/yamlHover.ts#L156 that should be updated to JSON.stringify(example, null, 4)

you only forgot about description for message definition

Copy link
Member

@derberg derberg left a comment

Choose a reason for hiding this comment

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

it works well in VSCode ❤️

examples are not super nice as they are stringified
Screenshot 2023-06-01 at 14 42 10

but it is because of yaml extension and https://github.com/redhat-developer/yaml-language-server/blob/main/src/languageservice/services/yamlHover.ts#L156 that should be updated to JSON.stringify(example, null, 4)

you only forgot about description for message definition

Copy link
Member

@derberg derberg left a comment

Choose a reason for hiding this comment

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

also missing description in definitions/2.6.0/license.json

definitions/2.6.0/components.json do not match spec https://www.asyncapi.com/docs/reference/specification/v2.6.0#componentsObject

and yeah, message

@@ -1,6 +1,6 @@
{
"type": "object",
"description": "An object representing a Server.",
"description": "An object representing a message broker, a server or any other kind of computer program capable of sending and/or receiving data",
Copy link
Member

Choose a reason for hiding this comment

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

there is longer description in the spec

@@ -11,6 +11,9 @@
}
]
},
"example": {
Copy link
Member

Choose a reason for hiding this comment

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

the description of servers do not match spec

Copy link
Member

@derberg derberg left a comment

Choose a reason for hiding this comment

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

also missing description in definitions/2.6.0/license.json

definitions/2.6.0/components.json do not match spec https://www.asyncapi.com/docs/reference/specification/v2.6.0#componentsObject

and yeah, message

@derberg derberg changed the title feat: added extensive descriptions and examples to the schema (v2.6.0) feat: add extensive descriptions and examples to the schema v2.6.0 Jun 1, 2023
Copy link
Member

@derberg derberg left a comment

Choose a reason for hiding this comment

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

super happy you added it

can't wait for you rolling it out to previous versions and then to the 3.0 ❤️

thanks 🍻

@derberg
Copy link
Member

derberg commented Jun 1, 2023

/rtm

@asyncapi-bot asyncapi-bot merged commit abc15de into asyncapi:master Jun 1, 2023
@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 5.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jun 1, 2023

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
0.0% 0.0% Duplication

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants