-
Notifications
You must be signed in to change notification settings - Fork 193
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
Print out the name of the signalFailure reason instead of just its enum value #186
Conversation
…um code And add unit test for the string conversion Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
94dc5b0
to
64f6ed2
Compare
@tfoote for review please :) The automatic build seems to be using some out of date dependencies unrelated to this change. |
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, couple nits.
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.
This looks generally fine to me, but I'll defer to @tfoote for final review.
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.
🥇
@tfoote can we get a review on this please. |
@tfoote friendly ping :) |
@tfoote any thoughts? |
@ros-pull-request-builder retest this please |
Print a slightly more informative error message from signalFailure in the tf2 MessageFilter and add a unit test for the string conversion.
First half of follow-through for #130
Part of fix for #118
Part of https://github.com/ros-security/aws-roadmap/issues/74
Signed-off-by: Emerson Knapp emerson.b.knapp@gmail.com