-
-
Notifications
You must be signed in to change notification settings - Fork 60
feat: add ros2 bindings #582
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
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.
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.
Add bindingVersion
|
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
Approved Do you have protocol description and logo? I'm collecting our schemas under one roof to patch them for various range of external tools So I want to add your bindings and create proper OG image. Like this |
Protocol description: The Robot Operating System (ROS) is a set of software libraries and tools that help you build robot applications. From drivers to state-of-the-art algorithms, and with powerful developer tools, ROS has what you need for your next robotics project. And it's all open source. Full project details on ROS.org Logo: https://www.ros.org/imgs/ROSPressKit.zip --> Any inside in the "ROS2 Logos" folder. |
Is there anything pending here @Pakisan? 😄 |
Sorry, lost focus. Thought that everything is ok and will be merged automatically Looking what's went wrong |
|
@amparo-siemens MR was patched Check it out and after that it will be ready to merge fyi: @derberg @char0n @jonaslagoni |
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
It looks good to me, thanks for your help |
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.
🤙
@amparo-siemens merged Welcome on board 🚀 |
🎉 This PR is included in version 6.9.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
@Pakisan @jonaslagoni hey, as I see here, the PR adds schemas to v3 which was released 2y ago. We need v3.1 release for that. Look into the main issue in the specification repo: asyncapi/spec#1109. This release should actually be reverted. |
Not sure that understand it. Bindings Object it's something that can be extended and cannot affect specification versions because channels, servers, operations or messages were affected Am I wrong? |
I agree with @Pakisan tbh 🤔 Not sure if we have anything in the spec saying it explicitly? 😄 |
But this is a completely new binding example, server bindings: https://www.asyncapi.com/docs/reference/specification/v3.0.0#serverBindingsObject
@jonaslagoni well spec doesn't say explicitly which parts, if modified, should trigger release, but then you could say that if we add new field to for example Message Object then it also should not trigger a release of the spec 😃 We do have a definition of what is a breaking change and what is not a breaking change: https://github.com/asyncapi/spec/blob/master/CONTRIBUTING.md#breaking-change-vs-non-breaking-change. in the past every new binging was a minor release: |
Yep, it's new binding. But our docs tells that binding are not mandatory and vendor may or may not support all of them Does it mean that every new version of already existed binding will trigger new release? In case of changing core semantics new release is needed. If we change operation by adding new field or removing it or extending it we will change semantics and new release will be needed I'm not arguing, I'm just trying to understand better how we evolve our specification in case of changes not directly related with core structure |
bindings are not mandatory like many other properties/fields in the spec - this has nothing to do with "release or not release".
no, bindings releases are independent, but first, binding must be added to the spec. AsyncAPI specification do not contain details of specific binding, so we are cool here. in other words:
|
I guess it makes sense 😄 |
@derberg understood I'll rollback this release and create new version using instructions in readme |
awesome, and don't merge of course the new release until we are ready with the spec. @fmvilas was main reviewer and he was out the whole July so he needs few days to catch up. Also I'm out next week. So we need to sync, catchup if anything is missing and plan the release cycle. I'd also love to have a call with someone from Siemens (probably you @amparo-siemens or @gramss) as we need not only contribution of new binding, but actually maintainers to own the binding long term (typical maintainer job, answer to issues, review PRs). We need some solid representation as in the past we were letting in bindings for different tech, from corporations like Mulesoft and there is 0 commitment from their side (actually the only person we can count on is the ex employee of Salesforce that basically does it because of the open source spirit). I saw https://opensource.siemens.com/ so am pretty confident, we will just have "simple formality" discussion. But yeah, one thing at a time 😅 and it is vacation season 😅 and you know we all do it as volunteers 😅 |
@derberg tagged you in MR @amparo-siemens will be available from upstream channel in new upcoming version, but still available in my "nightly" channel (portal where I store tuned versions of our schemas for other tools and vendors) |
Thank you @derberg for clarifying the details with the release. I think there might be a few things that can grow into and at some point out of the Already, I liked the involvement of the ROS community and would like to see it as a shared effort in order to have not a single (tunnel) vision for the future of this binding. Maybe @Achllle might want to join this meeting with @derberg ? @amparo-siemens and I will sure participate! Enjoy the summer and see you in September! |
Description