-
Notifications
You must be signed in to change notification settings - Fork 43
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
Add gazebo gui events #148
Conversation
Signed-off-by: John Shepherd <[email protected]>
Signed-off-by: John Shepherd <[email protected]>
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.
The code checker is not happy:
https://github.com/ignitionrobotics/ign-gui/pull/148/checks?check_run_id=1433264752
This looks good to me. Just to add more context for future readers, the idea here is to define events in ign-gui
so they can be reused by multiple downstream projects like ign-gazebo
and ign-rviz
. This is also related to #68.
We should deprecate ignition::gazebo::gui::GuiEvents in favor of these events.
I also just realized we don't have unit tests for the events. This is especially important in this case, because there aren't plugins using these events that can be tested. @JShep1, sorry to increase the scope of this PR, but do you think you could add some tests? Let me know how I can help.
Signed-off-by: Louise Poubel <[email protected]>
I opened #150 with some suggestions. |
* Suggestions to #148 * more codecheck Signed-off-by: Louise Poubel <[email protected]>
Thank you! I just merged your changes in |
Codecov Report
@@ Coverage Diff @@
## ign-gui3 #148 +/- ##
============================================
+ Coverage 59.57% 60.16% +0.58%
============================================
Files 20 20
Lines 2471 2500 +29
============================================
+ Hits 1472 1504 +32
+ Misses 999 996 -3
Continue to review full report at Codecov.
|
Signed-off-by: John Shepherd [email protected]