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

Prevent default-constructed variants from holding a type #371

Merged
merged 1 commit into from
Aug 1, 2021

Conversation

adlarkin
Copy link
Contributor

@adlarkin adlarkin commented Jul 30, 2021

Signed-off-by: Ashton Larkin ashton@openrobotics.org

🦟 Bug fix

Summary

This is a follow-up to #370 (see that PR for a description of the bug fix). Since #370 breaks ABI and we cannot make this change in released versions without changing behavior and potentially breaking downstream users (relevant discussion: #370 (comment)), this fix is targeted for the next unreleased version, which is Fortress.

I also added some missing test coverage/updated documentation related to #358.

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

@adlarkin adlarkin mentioned this pull request Jul 30, 2021
@github-actions github-actions bot added the 🏯 fortress Ignition Fortress label Jul 30, 2021
@codecov
Copy link

codecov bot commented Jul 30, 2021

Codecov Report

Merging #371 (e2a3fee) into main (742bd27) will increase coverage by 0.01%.
The diff coverage is n/a.

❗ Current head e2a3fee differs from pull request most recent head 16014e7. Consider uploading reports for the commit 16014e7 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main     #371      +/-   ##
==========================================
+ Coverage   58.14%   58.16%   +0.01%     
==========================================
  Files         170      170              
  Lines       16788    16788              
==========================================
+ Hits         9762     9764       +2     
+ Misses       7026     7024       -2     
Impacted Files Coverage Δ
include/ignition/rendering/Node.hh 100.00% <ø> (ø)
...e/ignition/rendering/base/BaseGaussianNoisePass.hh 96.66% <0.00%> (-3.34%) ⬇️
ogre2/src/Ogre2GpuRays.cc 95.61% <0.00%> (+0.57%) ⬆️

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 742bd27...16014e7. Read the comment docs.

@adlarkin adlarkin requested a review from chapulina July 30, 2021 17:40
Copy link
Contributor

@iche033 iche033 left a comment

Choose a reason for hiding this comment

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

looks good to me.

Migration.md Show resolved Hide resolved
Signed-off-by: Ashton Larkin <ashton@openrobotics.org>
@adlarkin adlarkin force-pushed the adlarkin/no_data_variant branch from 7ba8fce to 16014e7 Compare July 31, 2021 02:54
@adlarkin adlarkin merged commit 16014e7 into main Aug 1, 2021
@adlarkin adlarkin deleted the adlarkin/no_data_variant branch August 1, 2021 02:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏯 fortress Ignition Fortress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants