Skip to content
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

WorldControl: use event by default #531

Merged
merged 3 commits into from
Aug 7, 2023
Merged

Conversation

scpeters
Copy link
Member

@scpeters scpeters commented Apr 8, 2023

🎉 New feature

Changes a default behavior to resolve a TODO comment

Summary

I noticed a comment in WorldControl.cc (originally from #306) suggesting that a default behavior should be changed in gz-gui7, but that has already been released, so I'm proposing the change for gz-gui8. The change will cause events to be used by default instead of services. I am not familiar with any of the context for this but have opened a draft pull request to promote discussion.

Test it

Checklist

  • Signed all commits for DCO
  • Added tests
  • Added example and/or tutorial
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
@github-actions github-actions bot added the 🎵 harmonic Gazebo Harmonic label Apr 8, 2023
@codecov
Copy link

codecov bot commented Apr 8, 2023

Codecov Report

Merging #531 (a85e581) into main (82aa5b1) will decrease coverage by 0.02%.
Report is 2 commits behind head on main.
The diff coverage is n/a.

❗ Current head a85e581 differs from pull request most recent head 5512c92. Consider uploading reports for the commit 5512c92 to get more accurate results

@@            Coverage Diff             @@
##             main     #531      +/-   ##
==========================================
- Coverage   70.09%   70.07%   -0.02%     
==========================================
  Files          46       46              
  Lines        5062     5062              
==========================================
- Hits         3548     3547       -1     
- Misses       1514     1515       +1     
Files Changed Coverage Δ
src/plugins/world_control/WorldControl.cc 72.60% <ø> (ø)

... and 1 file with indirect coverage changes

@scpeters scpeters marked this pull request as ready for review June 2, 2023 01:13
@scpeters scpeters requested a review from jennuine as a code owner June 2, 2023 01:13
@jennuine
Copy link
Contributor

@scpeters can you look into why ci is failing?

@scpeters
Copy link
Member Author

CI seems to be failing already on the main branch

I ran the tests locally on my Mac laptop and they all passed. I'm guessing there are issues with the display / GPU in the cloud CI machines

@azeey azeey added the beta Targeting beta release of upcoming collection label Jul 31, 2023
@scpeters scpeters merged commit f715d6c into main Aug 7, 2023
2 of 4 checks passed
@scpeters scpeters deleted the scpeters/world_control_event_8 branch August 7, 2023 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta Targeting beta release of upcoming collection 🎵 harmonic Gazebo Harmonic
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants