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

Build with c++11 flag #13

Closed
wants to merge 1 commit into from
Closed

Build with c++11 flag #13

wants to merge 1 commit into from

Conversation

springmeyer
Copy link
Contributor

Unless c++14 is absolutely needed, we should stick to building/testing/supporting C++11.

I took a look at whether all the tests would build and pass in C++11. They do - the only thing requiring C++14 was the #include <mapbox/geometry.hpp> that indirectly pulled in mapbox/geometry/feature.hpp which pulls in std::experimental::optional from https://github.com/mapbox/geometry.hpp/blob/b0e41cc5635ff8d50e7e1edb73cadf1d2a7ddc83/include/mapbox/geometry/feature.hpp#L11.

Ideally, in my view geometry.hpp would be configured in such a way that std::experimental::optional is not forced on downstream users. Until this is fixed, simply using #include <mapbox/geometry/geometry.hpp> solves the problem elegantly and allows us to build in C++11 mode.

@springmeyer springmeyer added this to the v1.0.0 milestone Oct 16, 2017
@springmeyer springmeyer mentioned this pull request Oct 16, 2017
@artemp
Copy link
Contributor

artemp commented Oct 16, 2017

@springmeyer -

Unless c++14 is absolutely needed, we should stick to building/testing/supporting C++11.

We might or might not require c++14 support. My judgement is we will need it sooner rather than later. By sticking to c++11 we'll create user expectations which would be difficult to negotiate later.
There are of course issues with feature.hpp using experimental::optional and it should be possible to use just geometry.hpp without feature.hpp so lets fix them rather than starting with patching up and leaving unresolved issues for later.

@springmeyer
Copy link
Contributor Author

Sounds good @artemp - Will close this now. Noting however that I see this PR also fixed a few places were we were missing includes. So I propose we fix that separately. I've ticketed at #20

@springmeyer springmeyer closed this Nov 7, 2017
@springmeyer springmeyer deleted the c++11 branch November 7, 2017 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants