-
Notifications
You must be signed in to change notification settings - Fork 260
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
Don't crash when type definition cannot be found #1350
Conversation
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
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.
Gist: https://gist.githubusercontent.com/emersonknapp/0765b85b4298c80c570196adb92faa35/raw/cb4ebfa636908f3e5e1d6d4ad8f5a5d3d1c135da/ros2.repos |
@Mergifyio backport iron |
✅ Backports have been created
|
* Don't fail when type definition cannot be found Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com> (cherry picked from commit dae4660)
Given that humble is going to be around for a while as LTS, should this also be backported? |
@russkel Good instincts! It would be a yes, except the feature set |
How unfortunate. Presumably that's not a trivial merge then? |
Typically significant API changes (and therefore large new features) are very hard to backport, mostly we'll only do minor feature extensions and bugfixes. I did just remember while looking at it, that Humble does have the |
This pull request has been mentioned on ROS Discourse. There might be relevant details there: https://discourse.ros.org/t/ros-2-tsc-meeting-minutes-2023-06-15/32038/1 |
Fail gracefully to an empty type definition, if the name of the type was not fully understood by the local message definition source file search.
This is a fix to the crashing aspect of #1336 and #1286, which will downgrade the issue to "incomplete information in bag", which we have decided is a better outcome than a crash.