controller manager: enclose controller name in quotes #2761
Conversation
|
Seriously. I add 2 single-quotes and the format check is suddenly failing? |
So empty controller names are easier to spot.
gavanderhoorn
force-pushed
the
quotes_around_ctrlr_names
branch
from
c7f95a7
to
6590613
Compare
| << result->error_string); | ||
| ROS_WARN_STREAM_NAMED(LOGNAME, "Controller '" << name_ << "' failed with error " | ||
| << errorCodeToMessage(result->error_code) << ": " | ||
| << result->error_string); |
There was a problem hiding this comment.
| << result->error_string); | |
| << result->error_string.empty() ? state.getText() : result->error_string); |
maybe? maybe not?
There was a problem hiding this comment.
I already fixed it. The formatting job shows the suggested formatting. The << needed to be moved to the right by 1 space.
There was a problem hiding this comment.
this wasn't about the formatting but about displaying the generic action error string in case the other one is empty
There was a problem hiding this comment.
/friday_afternoon.
Sorry.
There was a problem hiding this comment.
Should probably be added in a separate PR though.
There was a problem hiding this comment.
Hehe, let's not micro-PR things? It's the same topic after all
There was a problem hiding this comment.
Hehe, let's not micro-PR things? It's the same topic after all
In theory I agree with @simonschmeisser, but I understand why we should not delay PRs because of other issues in the same code lines. Please create a quick follow-up PR for your improvement Simon! 🏅
|
Windows CI is broken afaik. |
Codecov Report
@@ Coverage Diff @@
## master #2761 +/- ##
==========================================
- Coverage 60.45% 60.44% -0.01%
==========================================
Files 404 404
Lines 29670 29670
==========================================
- Hits 17934 17931 -3
- Misses 11736 11739 +3
Continue to review full report at Codecov.
|
You lost some vowels there ;) |
So empty controller names are easier to spot.
Some MoveIt configurations have the controller
nameset to"". This is valid (although annoying), but the formatting of the log lines makes it difficult to spot this.Without the quotes:
With the quotes: