-
-
Notifications
You must be signed in to change notification settings - Fork 269
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 googlepubsub bindings #836
Conversation
There was a problem hiding this 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.
Added as pending candidate for 2.5 release - #830 |
@whitlockjc please also mention it under other bindings, even if for now they do not contain any official binding definition. It is for consistency, and to keep the word reserved for the same protocol along all bindings. Have a look at also please list |
I'm not sure I follow. Does adding Google Cloud Pub/Sub bindings require a relating protocol? I assume that adding a new protocol would mean modifying the JSON Schemas PR as well? |
082d967
to
2f0fb3b
Compare
Done. |
Adds support for Google Cloud Pub/Sub bindings. See: asyncapi/bindings#141
2f0fb3b
to
c6c0bad
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Kudos, SonarCloud Quality Gate passed! |
@char0n I think we are ready to merge this one. There was one merge conflict I had to solve manually, it was related to recent IBM MQ bug fix, addition of missing binding, and it was conflicting with new googlepubsub |
@derberg I'm a bit worried about the timing here. Merging this work will trigger rounds of work that would need to be done. Given that we're just days before the release deadline I would opt postponing (I'm not sure If I have this option) this work to the next release. As far as I understand this would require providing implementation to parser-js, right @magicmatatjahu? What do you think guys? |
@char0n there is no need to do anything in parser-js, so we just need to merge this one and asyncapi/spec-json-schemas#253. There is also no need for any followup implementation in other AsyncAPI tools |
Yeah, as @derberg wrote, due to fact that bindings are "dynamic" objects and their shape is determined by binding's version, we treat bindings in tooling as raw JSON data - we don't have any models/api for bindings. |
@char0n so, are we merging? asyncapi/spec-json-schemas#253 is approved, so all ready |
@derberg @magicmatatjahu thanks guys for explanations. Merging. |
/rtm |
🎉 This PR is included in version 2.5.0-next-spec.4 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Adds support for Google Cloud Pub/Sub to the
Channel Binding Object
andMessage Binding Object
lists.Related issue(s):
Bindings PR: asyncapi/bindings#141
JSON Schema PR: asyncapi/spec-json-schemas#253