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

Fix const-correctness of the Model::JointByName and Model::LinkByName APIs #2059

Merged
merged 1 commit into from
Jul 28, 2023

Conversation

arjo129
Copy link
Contributor

@arjo129 arjo129 commented Jul 28, 2023

🦟 Bug fix

Fixes #

Summary

This has been a longstanding pet-peeve of mine and a todo in the code base. Unfortunately, this change likely breaks ABI and hence can only be done in a major release. It does not however break API and hence old code should not have any issues compiling against this new change.

An alternative would be to introduce these const functions as duplicates in citadel and deprecate the non-const functions in garden, but this feels like more work.

Checklist

  • Signed all commits for DCO
  • Added tests
  • 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.

🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸

…ame` APIs

This has been a longstanding pet-peeve of mine and a todo in the code
base. Unfortunately, this change likely breaks ABI and hence can only
be done in a major release.

Signed-off-by: Arjo Chakravarty <arjoc@intrinsic.ai>
@arjo129 arjo129 requested a review from mjcarroll as a code owner July 28, 2023 02:22
@github-actions github-actions bot added the 🎵 harmonic Gazebo Harmonic label Jul 28, 2023
@codecov
Copy link

codecov bot commented Jul 28, 2023

Codecov Report

Merging #2059 (5df2724) into main (dbebc39) will increase coverage by 0.00%.
Report is 1 commits behind head on main.
The diff coverage is 100.00%.

❗ Current head 5df2724 differs from pull request most recent head 4d8c788. Consider uploading reports for the commit 4d8c788 to get more accurate results

@@           Coverage Diff           @@
##             main    #2059   +/-   ##
=======================================
  Coverage   65.45%   65.45%           
=======================================
  Files         312      312           
  Lines       29808    29810    +2     
=======================================
+ Hits        19510    19512    +2     
  Misses      10298    10298           
Files Changed Coverage Δ
include/gz/sim/components/Component.hh 100.00% <ø> (ø)
src/Model.cc 96.15% <ø> (ø)
include/gz/sim/components/Factory.hh 97.29% <100.00%> (+0.02%) ⬆️
src/ComponentFactory.cc 100.00% <100.00%> (ø)

@arjo129 arjo129 merged commit 29a58c5 into main Jul 28, 2023
4 of 5 checks passed
@arjo129 arjo129 deleted the arjo/fix/harmonic_model_const_correctness branch July 28, 2023 04:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎵 harmonic Gazebo Harmonic
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants