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 protocol binding schemas #225

Conversation

jonaslagoni
Copy link
Member

Description
This PR adds the JSON schema files from the bindings repository.

Things to figure out in this PR:

  • Where should we place these files?
    • We could have them in /bindings/protocol, however, how do you reference specification extension? While they rarely, if ever change, using https://raw.githubusercontent.com/asyncapi/asyncapi-node/v2.7.7/schemas/2.0.0.json#/definitions/specificationExtension seems wrong in some sense, if it's part of spec v3 🤔

Things to figure out after this PR:

  • How to version the binding schemas alongside spec schemas

Related issues
Partly solves asyncapi/bindings#113

@sonarcloud
Copy link

sonarcloud bot commented May 27, 2022

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

jonaslagoni commented Jun 9, 2022

Thanks @dalelane 🙏

/rtm

@asyncapi-bot asyncapi-bot merged commit 7d396cf into asyncapi:next-major-spec Jun 9, 2022
@derberg
Copy link
Member

derberg commented Jun 9, 2022

@jonaslagoni why does it go to next-major-spec? why not master?

@jonaslagoni
Copy link
Member Author

@jonaslagoni why does it go to next-major-spec? why not master?

It was honestly more on the cautious side I choose to target that branch. Mainly because we still have no idea how to actually do the setup yet. So pairing it with a distant release in the future where we have time to figure out the right approach made the most sense 🙂

@jonaslagoni jonaslagoni deleted the feature/add_all_binding_schemas branch June 9, 2022 11:10
@derberg
Copy link
Member

derberg commented Jun 9, 2022

makes sense, but why not next-major instead? integrating binding schemas are not related to spec evolution, but just the json schema evolution

@jonaslagoni
Copy link
Member Author

makes sense, but why not next-major instead? integrating binding schemas are not related to spec evolution, but just the json schema evolution

Hmm, that could have worked as well yes, did not really think about it.

An upside to having it in the next-major-spec branch is it will be used more or less right out the gates because the parser already has a specific branch for this. But we could do that for next-major as well, so yea 🤷

It will be more convenient for me to work on it from the next-major-spec branch, but correctly, it should probably reside in next-major 🤔 Up to you 🙂

@derberg
Copy link
Member

derberg commented Jun 9, 2022

next-major is also supported out of the box. Branch and config is there, both for this project and parser.

So in the end it is up to you not me as you do the work on integrating bindings, you know what is your target. I just think it would be beneficial to introduce it earlier as yeah 3.0 is not happening too soon.

@smoya
Copy link
Member

smoya commented Jul 8, 2022

@jonaslagoni @derberg Shall we also serve these via https://asyncapi.com/bindings as we do for schemas and definitions?

@jonaslagoni
Copy link
Member Author

@jonaslagoni @derberg Shall we also serve these via https://asyncapi.com/bindings as we do for schemas and definitions?

I would say so yes 🙂

@derberg
Copy link
Member

derberg commented Jul 11, 2022

yup, there are just not yet integrated with main schema

@smoya
Copy link
Member

smoya commented Jul 11, 2022

@jonaslagoni @derberg Shall we also serve these via https://asyncapi.com/bindings as we do for schemas and definitions?

I created a follow up issue on this @jonaslagoni @derberg asyncapi/website#870

@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 3.2.0-next-major-spec.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 5.0.0-next-major-spec.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@asyncapi-bot
Copy link
Contributor

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

The release is available on:

Your semantic-release bot 📦🚀

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