Conversation
|
@tonynajjar, please properly fill in PR template in the future. @SteveMacenski, use this instead.
|
|
@tonynajjar, your PR has failed to build. Please check CI outputs and resolve issues. |
SteveMacenski
left a comment
There was a problem hiding this comment.
LGTM, just unavoidable linter complaints
| void | ||
| LifecycleManager::CreateActiveDiagnostic(diagnostic_updater::DiagnosticStatusWrapper & stat) | ||
| { | ||
|
|
There was a problem hiding this comment.
White space that I'm sure CI will complain about - no empty lines at the start or end of the function
There was a problem hiding this comment.
Weird, I ran ament_uncrustify
root@10a5941ce854:~# ament_uncrustify /root/ros2_ws/src/navigation2/nav2_lifecycle_manager/src/lifecycle_manager.cpp
No code style divergence in file '/root/ros2_ws/src/navigation2/nav2_lifecycle_manager/src/lifecycle_manager.cpp'
No problems found
Anything else I should run to detect the CI problems you mentioned?
There was a problem hiding this comment.
You may have an outdated version 🤷
|
Test lifecycle also failed: https://app.circleci.com/pipelines/github/ros-planning/navigation2/7390/workflows/720796c3-4123-4fed-a865-8d126264caf3/jobs/26044 that should not be failing, so this might introduce a regression |
|
I'm not 100% sure but it seems like the nav2_lifecycle_manager tests pass now? I simply fixed the empty lines. Would be nice to see a more detailed summary (or I'm missing where to look):
|
|
Yeah maybe, I'm going to retrigger CI a few times just to make sure There's a more detailed look in the artifacts tab, sorted by stdout in each package's tests |
|
Ran 4x, looked related to the linter error, done! |
* add diagnostics * fix * fix
Basic Info
Description of contribution in a few bullet points
Description of documentation updates required from your changes
Future work that may be required in bullet points
For Maintainers: