-
Notifications
You must be signed in to change notification settings - Fork 67
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
Use ImplPtr from ign-utils where relevant #299
Conversation
@osrf-jenkins retest this please |
Signed-off-by: Michael Carroll <[email protected]>
Signed-off-by: Michael Carroll <[email protected]>
Signed-off-by: Michael Carroll <[email protected]>
Signed-off-by: Michael Carroll <[email protected]>
e96a2e8
to
b59f8e2
Compare
I've added an ignition-math7 formula in osrf/homebrew-simulation#1727, so it should be easy now to add ignition-utils as a dependency |
Codecov Report
@@ Coverage Diff @@
## main #299 +/- ##
==========================================
+ Coverage 99.53% 99.64% +0.10%
==========================================
Files 68 65 -3
Lines 6275 6131 -144
==========================================
- Hits 6246 6109 -137
+ Misses 29 22 -7
Continue to review full report at Codecov.
|
Signed-off-by: Michael Carroll <[email protected]>
Signed-off-by: Michael Carroll <[email protected]>
Signed-off-by: Michael Carroll <[email protected]>
@scpeters this is ready for another look |
Signed-off-by: Michael Carroll <[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.
Mainly minor comments
Signed-off-by: Michael Carroll <[email protected]>
1bffd38
to
15e65e3
Compare
Signed-off-by: Michael Carroll <[email protected]>
🎉 New feature
We are introducing ign-utils as a dependency, so it seems like a reasonable to to switch to ImplPtr where relevant. I have done this for everything except for what is covered by #294
Note to maintainers: Remember to use Squash-Merge