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

Updated tests for ign-math's ostream fix #847

Merged
merged 1 commit into from
Feb 18, 2022
Merged

Conversation

jennuine
Copy link
Collaborator

@jennuine jennuine commented Feb 9, 2022

🦟 Bug fix

Summary

With changes from the above ign-math PR, the output stream does not change the value. This change affected how magneticField value is printed:

math::Vector3d magneticField(5.5645e-6, 22.8758e-6, -42.3884e-6);
cout << magneticField;

// before printed:   6e-06 2.3e-05 -4.2e-05
// after ign-math PR: 5.5645e-06 2.28758e-05 -4.23884e-05

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • 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: Jenn Nguyen <[email protected]>
@jennuine
Copy link
Collaborator Author

@osrf-jenkins run tests please

@codecov-commenter
Copy link

codecov-commenter commented Feb 18, 2022

Codecov Report

Merging #847 (48de1b0) into main (1df5df2) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #847   +/-   ##
=======================================
  Coverage   90.69%   90.69%           
=======================================
  Files          78       78           
  Lines       12433    12433           
=======================================
  Hits        11276    11276           
  Misses       1157     1157           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1df5df2...48de1b0. Read the comment docs.

@azeey
Copy link
Collaborator

azeey commented Feb 18, 2022

We need to forward port sdf12 changes to main for CI to pass. I'll go ahead and do that.

@azeey
Copy link
Collaborator

azeey commented Feb 18, 2022

We need to forward port sdf12 changes to main for CI to pass. I'll go ahead and do that.

I started doing this, but tests fail without the changes in this PR to account for gazebosim/gz-math#376. I am thinking of merging this as is and opening the forward port PR immediately, which will make CI green again. What do you think @scpeters ?

@scpeters
Copy link
Member

We need to forward port sdf12 changes to main for CI to pass. I'll go ahead and do that.

I started doing this, but tests fail without the changes in this PR to account for ignitionrobotics/ign-math#376. I am thinking of merging this as is and opening the forward port PR immediately, which will make CI green again. What do you think @scpeters ?

sounds good to me

@azeey azeey merged commit 54188ff into main Feb 18, 2022
@azeey azeey deleted the jennuine/ostream_fix branch February 18, 2022 21:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌱 garden Ignition Garden
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants