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

Use Boost.Variant instead of GIL's own variant implementation. #231

Merged
merged 4 commits into from
Feb 5, 2019

Conversation

sdebionne
Copy link
Contributor

@sdebionne sdebionne commented Feb 4, 2019

Tentatively trying to fix #131 and working around GCC bug 87107

Since Boost.Variant already supports lambda visitors, this makes #227 probably unnecessary.

References

#131
#227

Tasklist

  • Review
  • Adjust for comments
  • All CI builds and checks have passed

@sdebionne sdebionne changed the title WIP: Use Boost.Variant instead of GIL's own variant implementation. Use Boost.Variant instead of GIL's own variant implementation. Feb 4, 2019
@mloskot mloskot added the cat/enhancement Improvements, but not fixes addressing identified bugs label Feb 4, 2019
Copy link
Member

@mloskot mloskot 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. No new warnings seem to appear (at least on Travis), that's good.
There is a trade-off obviously and GIL now directly requires Boost.Variant, but I think the benefits outweight that extra dep.

@sdebionne Thanks for the contribution!

If this is merged, I think it should close #131 as fixed :-)

Let's see what @stefanseefeld says

@mloskot mloskot added this to the Boost 1.70 milestone Feb 5, 2019
@mloskot mloskot merged commit 2308a1a into boostorg:develop Feb 5, 2019
@mloskot
Copy link
Member

mloskot commented Feb 5, 2019

I made mistake while editing squashed commits message, this should read C++14 in

Adds support C++11 decltype(auto)

I don't feel comfy about amending the pushed commit. Sorry about that.

@mloskot mloskot added ext/dynamic_image boost/gil/extension/dynamic_image cat/annoyance Not a bug, not a feature, but something that should be improved labels Feb 5, 2019
@mloskot
Copy link
Member

mloskot commented Feb 5, 2019

@stefanseefeld, @sdebionne
While adding CI build w/ C++14, #233, I realised, are we having any tests for the dynamic_image extension or I'm just not seeing them?

I think the only executable compiled with any #include <boost/gil/extension/dynamic_image/*> is the example/dynamic_image.cpp.

@stefanseefeld
Copy link
Member

gil/test/image.cpp seems to contain some tests involving the dynamic_image extension.

@sdebionne sdebionne deleted the fix-apply-operation branch February 6, 2019 14:00
@mloskot
Copy link
Member

mloskot commented Feb 7, 2019

@stefanseefeld Right. Let's take it for now as enough :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cat/annoyance Not a bug, not a feature, but something that should be improved cat/enhancement Improvements, but not fixes addressing identified bugs ext/dynamic_image boost/gil/extension/dynamic_image
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Compilation of extension/dynamic_image/apply_operation_base.hpp takes ages with GCC 7 or 8
3 participants