-
-
Notifications
You must be signed in to change notification settings - Fork 74
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: adapt googlepubsub bindings to v3 #213
feat: adapt googlepubsub bindings to v3 #213
Conversation
0f687a8
to
9060b2d
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.
please remember to bump the bindingVersion
, in JSON Schemas too
9060b2d
to
ae9951f
Compare
Changes from v2 to v3: * Bumped the `bindingVersion` values to `0.2.0` * The `topic` property for the Channel Binding Object is no more and is now provided via the native `address` property of the Channel Object * Protobuf support now uses the native support by providing the Protobuf definition as an inline string of the `schema` property within the `payload` property of the Message Object and the `schemaFormat` property within the `payload` property of the Message Object now has valid values for Protobuf 2 and 3 * The `type` property for the Message Binding Object is no more as AsyncAPI now natively support Protobuf and this value would duplicate the `schemaFormat` in the `payload` property of the Message Object
ae9951f
to
629c9b8
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.
Awesome 👍
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.
Is the idea that someone using the binding for an AsyncAPI v2.x doc will keep using the v0.1.0 binding, while someone using the binding for an AsyncAPI v3.x doc will need to use the v0.2.0 binding?
Apologies - this is perhaps more of a general question about updating bindings than about this specific PR, but this is the first one that has got me to think about it!
Assuming this is true and I'm not going down a misunderstanding-tangent, should we call that out in the bindings README's? e.g.
I'm thinking that it will take time for people to migrate to v3, so things like this will help ease the path. |
@dalelane That's my understanding. |
Besides that, would it make sense to bump to a major version of the binding (1.0.0) instead of a minor one so it helps noticing "there is something that might break -- please check" ? |
I just followed the same pattern the other bindings made, each of which bumped the minor version and not the major version. Were there changes I made that warrant a major version bump? cc @derberg, @jonaslagoni |
If the binding is below v0 you can do however you like 🙂 For the other bindings, there did not seem to be any interest in marking them as stable (v1), so a minor bump was done. Even if the change is breaking, i.e. you cant just bump the binding version you use in your AsyncAPI document, you have to do some more work. |
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.
LGTM! 🚀🌔
/rtm |
Changes from v2 to v3:
bindingVersion
values to0.2.0
topic
property for the Channel Binding Object is no more and is now provided via the nativeaddress
property of the Channel Objectschema
property within thepayload
property of the Message Object and theschemaFormat
property within thepayload
property of the Message Object now has valid values for Protobuf 2 and 3type
property for the Message Binding Object is no more as AsyncAPI now natively support Protobuf and this value would duplicate theschemaFormat
in thepayload
property of the Message ObjectSee also #182.