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

Add NeverDestroyed utility class #31

Merged
merged 7 commits into from
Dec 9, 2021
Merged

Add NeverDestroyed utility class #31

merged 7 commits into from
Dec 9, 2021

Conversation

mjcarroll
Copy link
Contributor

🎉 New feature

Summary

Useful for function-local static variables that are not trivially destructable. This can be used to clean up "possible leak" warnings from memory-checking tools.

Originally imported from drake codebase (BSD 3-clause). It has been updated for consistency with ignition codebase.

Added in support of gazebosim/gz-math#269 at the suggestion of @jwnimmer-tri (gazebosim/gz-math#269 (comment))

Test it

Unit tests have been imported as well

Checklist

  • Signed all commits for DCO
  • Added tests
  • Added example and/or tutorial
  • 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

@mjcarroll mjcarroll requested a review from azeey as a code owner December 8, 2021 20:28
@mjcarroll mjcarroll self-assigned this Dec 8, 2021
@mjcarroll mjcarroll requested a review from scpeters December 8, 2021 20:28
@github-actions github-actions bot added 🌱 garden Ignition Garden 🏢 edifice Ignition Edifice 🏯 fortress Ignition Fortress labels Dec 8, 2021
@mjcarroll
Copy link
Contributor Author

@jwnimmer-tri based on your comment on the ign-math issue above, we have decided to move forward with this option for classes such as math::Material.

Would it be possible for you to sign off on relicensing this to Apache 2.0? While BSD is compatible, for the ease of packaging, it helps to have as few different licenses as possible. I have deferred adding any sort of license/copyright header until I get confirmation from you.

@jwnimmer-tri
Copy link

Yes, given the git history of the header and test code, TRI (me) are the sole author. TRI (and me) grant you license under Apache-2.0 terms to contribute it here.

include/ignition/utils/NeverDestroyed.hh Outdated Show resolved Hide resolved
include/ignition/utils/NeverDestroyed.hh Outdated Show resolved Hide resolved
include/ignition/utils/NeverDestroyed.hh Outdated Show resolved Hide resolved
src/NeverDestroyed_TEST.cc Show resolved Hide resolved
src/NeverDestroyed_TEST.cc Outdated Show resolved Hide resolved
mjcarroll and others added 4 commits December 9, 2021 09:12
Signed-off-by: Michael Carroll <[email protected]>
Signed-off-by: Michael Carroll <[email protected]>

Co-authored-by: Addisu Z. Taddese <[email protected]>
Signed-off-by: Michael Carroll <[email protected]>
Signed-off-by: Michael Carroll <[email protected]>
Signed-off-by: Addisu Z. Taddese <[email protected]>
@codecov-commenter
Copy link

Codecov Report

Merging #31 (ce7879f) into ign-utils1 (07cb45a) will increase coverage by 0.50%.
The diff coverage is 100.00%.

Impacted file tree graph

@@              Coverage Diff               @@
##           ign-utils1      #31      +/-   ##
==============================================
+ Coverage       94.36%   94.87%   +0.50%     
==============================================
  Files               4        5       +1     
  Lines              71       78       +7     
==============================================
+ Hits               67       74       +7     
  Misses              4        4              
Impacted Files Coverage Δ
include/ignition/utils/NeverDestroyed.hh 100.00% <100.00%> (ø)

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 07cb45a...ce7879f. Read the comment docs.

@mjcarroll mjcarroll enabled auto-merge (squash) December 9, 2021 18:25
@mjcarroll mjcarroll merged commit c3c71ce into ign-utils1 Dec 9, 2021
@mjcarroll mjcarroll deleted the never_destroyed branch December 9, 2021 18:38
@osrf-triage
Copy link

This pull request has been mentioned on Gazebo Community. There might be relevant details there:

https://community.gazebosim.org/t/new-ignition-releases-2022-03-01-citadel-edifice-fortress/1313/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏢 edifice Ignition Edifice 🏯 fortress Ignition Fortress 🌱 garden Ignition Garden
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants