Skip to content

ign -> gz Header Migration : ign-math#413

Merged
methylDragon merged 4 commits intomainfrom
header_migration
May 2, 2022
Merged

ign -> gz Header Migration : ign-math#413
methylDragon merged 4 commits intomainfrom
header_migration

Conversation

@methylDragon
Copy link
Copy Markdown
Contributor

@methylDragon methylDragon commented Apr 28, 2022

@methylDragon methylDragon requested a review from scpeters as a code owner April 28, 2022 17:23
@github-actions github-actions bot added the 🌱 garden Ignition Garden label Apr 28, 2022
@methylDragon methylDragon mentioned this pull request Apr 28, 2022
Closed
@chapulina chapulina added the ign to gz Renaming Ignition to Gazebo. label Apr 28, 2022
@methylDragon methylDragon marked this pull request as draft April 28, 2022 23:55
@methylDragon methylDragon force-pushed the header_migration branch 4 times, most recently from 1bc7e9a to 253963c Compare April 29, 2022 01:02
@methylDragon
Copy link
Copy Markdown
Contributor Author

methylDragon commented Apr 29, 2022

Extra files have been deleted.

I'm not sure if I edited the Bazel buildfile correctly.

scpeters
scpeters reviewed Apr 29, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@scpeters scpeters left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this also has some of the same issues as ign-utils:

  • remove ignition/math/math.hh redirection header
  • add ignition/math/detail/Export.hh redirection header

include/ignition/math/MassMatrix3.ipynb Outdated
@@ -0,0 +1,134 @@
{
Copy link
Copy Markdown
Member

@scpeters scpeters Apr 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this file should be deleted

Copy link
Copy Markdown
Contributor Author

@methylDragon methylDragon Apr 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

253963c !

Copy link
Copy Markdown
Contributor Author

@methylDragon methylDragon Apr 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noting: git picked up the math.hh deletion and Export creation as a move for some reason.

@methylDragon
Copy link
Copy Markdown
Contributor Author

methylDragon commented Apr 29, 2022

Ubuntu CI is throwing a warning (Python not found), I don't think this is related to this PR.

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 29, 2022 •
edited
Loading

Codecov Report

Merging #413 (9bf23b9) into main (9622e9e) will not change coverage.
The diff coverage is n/a.

❗ Current head 9bf23b9 differs from pull request most recent head f243dd3. Consider uploading reports for the commit f243dd3 to get more accurate results

@@           Coverage Diff           @@
##             main     #413   +/-   ##
=======================================
  Coverage   99.84%   99.84%           
=======================================
  Files          49       49           
  Lines        4396     4396           
=======================================
  Hits         4389     4389           
  Misses          7        7
Impacted Files Coverage Δ
...include/ignition/math7/ignition/math/detail/Box.hh
...tall/include/ignition/math7/ignition/math/Angle.hh
...nclude/ignition/math7/ignition/math/OrientedBox.hh
...ignition/math7/ignition/math/eigen3/Conversions.hh
...ll/include/ignition/math7/ignition/math/Matrix4.hh
.../include/ignition/math7/ignition/math/Triangle3.hh
...7/ignition/math/AdditivelySeparableScalarField3.hh
...l/include/ignition/math7/ignition/math/Interval.hh
...tall/include/ignition/math7/ignition/math/Plane.hh
...l/include/ignition/math7/ignition/math/Cylinder.hh
... and 82 more

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 9622e9e...f243dd3. Read the comment docs.

@methylDragon
Copy link
Copy Markdown
Contributor Author

methylDragon commented Apr 29, 2022

We just need #414 merged for CI to pass, I think.

@methylDragon methylDragon force-pushed the header_migration branch 2 times, most recently from 15016d3 to b6da548 Compare April 30, 2022 01:00
@methylDragon methylDragon marked this pull request as ready for review April 30, 2022 01:09
scpeters
scpeters reviewed Apr 30, 2022
View reviewed changes
include/ignition/math/eigen3.hh
@@ -0,0 +1,18 @@
/*
Copy link
Copy Markdown
Member

@scpeters scpeters Apr 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would have expected this file in the eigen/include/ignition/math folder

Copy link
Copy Markdown
Contributor Author

@methylDragon methylDragon May 2, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated! cf5dba7

@methylDragon
Move header files with git mv
85a8dcd 
Signed-off-by: methylDragon <methylDragon@gmail.com>
@methylDragon
Create redirection aliases
cf5dba7 
Signed-off-by: methylDragon <methylDragon@gmail.com>
@methylDragon
Migrate sources in src, test, examples, and include
d9ef5fe 
Signed-off-by: methylDragon <methylDragon@gmail.com>
@methylDragon
Migrate build files
f243dd3 
Signed-off-by: methylDragon <methylDragon@gmail.com>
@methylDragon methylDragon merged commit 9b7714c into main May 2, 2022
@methylDragon methylDragon deleted the header_migration branch May 2, 2022 21:39
@scpeters
Copy link
Copy Markdown
Member

scpeters commented May 3, 2022

I think we missed the headers from #412 when rebasing

@methylDragon methylDragon mentioned this pull request May 3, 2022
Merged
@scpeters scpeters mentioned this pull request May 9, 2022
Merged
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@scpeters scpeters scpeters approved these changes

Assignees

No one assigned

Labels

🌱 garden Ignition Garden ign to gz Renaming Ignition to Gazebo.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@methylDragon @scpeters @j-rivero @chapulina @osrf-triage