-
Notifications
You must be signed in to change notification settings - Fork 111
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
Fix SolidPrimitive.msg to contain a single Polygon #189
Conversation
Signed-off-by: M. Fatih Cırıt <[email protected]>
@clalancette the ABI won't be compatible but is it possible to include this fix in The worst case scenario I will add a comment line on the old message as a back-port clarification. Something like |
Today is the last possible moment to do this. I'm very much scared of doing this, but we'll discuss it internally and see what we want to do here. |
I see, I am so sorry 🙇♂️ for making this mistake and realizing it too late. Thank you so much for helping. |
Besides the CI, I also went through all of the packages released into Humble to see which ones could potentially be broken by this. It's actually a fairly short list of packages that use the SolidPrimitives message, the list looks to be:
I went through and looked at all of the uses here, and none of them seem to use the polygon or PRISM type. I also went ahead and built all of them locally with this patch applied, so I think we are OK to go forward with this. Still waiting on CI to finish, then we'll backport and release. |
@Mergifyio backport humble |
Signed-off-by: M. Fatih Cırıt <[email protected]> (cherry picked from commit dfb9991)
✅ Backports have been created
|
Signed-off-by: M. Fatih Cırıt <[email protected]> (cherry picked from commit dfb9991) Co-authored-by: M. Fatih Cırıt <[email protected]>
Follow up from #167
In my previous PR I was supposed to add a single
geometry_msgs::msg::Polygon
to the message but I've added an array of it by mistake 🤦♂️It's even a part of humble now :(
This PR fixes it.