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 all recommended and must supported schema formats #365

Merged

Conversation

jonaslagoni
Copy link
Member

Description
This PR adds all the MUST and RECOMMENDED supported schema formats. This is to enable auto-completion more than anything because the underlying type is string, it's only to help know which known values are there.

@sonarqubecloud
Copy link

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

@GreenRover
Copy link
Collaborator

If tooling will support proto but it is not listed here it is not a problem?

@jonaslagoni
Copy link
Member Author

These are just all known schema formats listed in https://www.asyncapi.com/docs/reference/specification/v2.6.0#messageObjectSchemaFormatTable

We cannot add values we haven't agreed upon, you can of course still use custom ones.

Adding new keys for other schema formats is part of the spec discussion.

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.

looks like manual stuff that will have to be updated with each release

not a problem, just mentioning because this requires updates in readme, and probably script that generates new version definitions?

@jonaslagoni
Copy link
Member Author

not a problem, just mentioning because this requires updates in readme, and probably script that generates new version definitions?

I'll see if I can include it in the script, that would be easiest yea.

@sonarqubecloud
Copy link

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

@jonaslagoni
Copy link
Member Author

Alright, I made a few changes.

  1. Adapted the create new version script:
  • Before, we could accidentally replace versions that were unintended, i.e. it would happen in the next version (3.1.0) because it would replace the application/vnd.oai.openapi;version=3.0.0 to application/vnd.oai.openapi;version=3.1.0.
  • If the new version is a major version change (i.e. first number is not equal to the latest), replace all AsyncAPI schema format entries with the new.
  • If the new version is minor or fix, add a new entry of AsyncAPI schema format.
  1. Adapted the root .asyncapi property to use const instead of enum. We will never have more then one value.
  2. Removed all v2 schema formats

Depending on whether this or #370 is merged first, we need to adapt the function addNewSchemaVersion.

@jonaslagoni jonaslagoni requested a review from derberg June 29, 2023 14:31
definitions/3.0.0/messageObject.json Outdated Show resolved Hide resolved
definitions/3.0.0/messageObject.json Outdated Show resolved Hide resolved
@jonaslagoni
Copy link
Member Author

Applied your suggestions @fmvilas and applied the recent changes from master.

I adapted the add new version script so the validation of schema property is also changed accordingly with the right schema format values.

@jonaslagoni jonaslagoni requested a review from fmvilas July 25, 2023 09:37
@jonaslagoni
Copy link
Member Author

Please have another look @fmvilas @derberg 🙇

@sonarqubecloud
Copy link

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

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.

LGTM 🚀

tested on local 👇🏼

Screenshot 2023-07-25 at 14 55 28
Screenshot 2023-07-25 at 14 56 14

@jonaslagoni
Copy link
Member Author

jonaslagoni commented Jul 25, 2023

@derberg did you test the auto-suggestion feature for schemaFormat values, not so much whether schema is validated? 😄

Copy link
Member

derberg commented Jul 26, 2023

you mean this 👇

Screenshot 2023-07-26 at 16.14.52.png

@jonaslagoni
Copy link
Member Author

@derberg yeeep, perfect 👌

@jonaslagoni
Copy link
Member Author

/rtm

Copy link
Member

derberg commented Jul 26, 2023

@fmvilas is out on holidays next 4 weeks, and for not PR is blocked as he requested changes to it in the past.

I will use admin rights to dismiss his request for changes as we won't wait a month for him

@derberg derberg merged commit ba244f8 into asyncapi:next-major-spec Jul 26, 2023
@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 6.0.0-next-major-spec.5 🎉

The release is available on:

Your semantic-release bot 📦🚀

@jonaslagoni jonaslagoni deleted the feature/add_concrete_schema_formats branch July 26, 2023 15:03
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.

5 participants