-
Notifications
You must be signed in to change notification settings - Fork 276
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
Remove playback <path> SDF param in Dome #570
Conversation
Signed-off-by: Mabel Zhang <[email protected]>
Signed-off-by: Mabel Zhang <[email protected]>
Signed-off-by: Mabel Zhang <[email protected]>
Signed-off-by: Mabel Zhang <[email protected]>
Signed-off-by: Mabel Zhang <[email protected]>
Signed-off-by: Mabel Zhang <[email protected]>
Signed-off-by: Mabel Zhang <[email protected]>
…cs/ign-gazebo into remove_playback_path_dome
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.
Looks good, just have a comment on the test regression
Signed-off-by: Mabel Zhang <[email protected]>
…cs/ign-gazebo into remove_playback_path_dome
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 pending CI
Codecov Report
@@ Coverage Diff @@
## ign-gazebo4 #570 +/- ##
===============================================
+ Coverage 77.29% 77.35% +0.05%
===============================================
Files 208 213 +5
Lines 11198 11925 +727
===============================================
+ Hits 8655 9224 +569
- Misses 2543 2701 +158
Continue to review full report at Codecov.
|
@osrf-jenkins run tests please |
Not sure why tests are failing, possibly flaky tests, re-running CI |
All green except ServerRepeat/SceneBroadcasterTest.State/0, which is flaky: https://build.osrfoundation.org/job/ignition_gazebo-ci-pr_any-ubuntu_auto-amd64/5269/testReport/(root)/ServerRepeat_SceneBroadcasterTest/State_0/history/ Merging... if I can figure out which merge button to push. |
Address #293 for Dome
<path>
SDF parameter forLogPlayback
<path>
to<playback_path>
to disambiguate between the two, similar to what's already done for<record_path>