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 feature id support #60

Merged
merged 3 commits into from
Feb 7, 2017
Merged

Add feature id support #60

merged 3 commits into from
Feb 7, 2017

Conversation

jfirebaugh
Copy link
Contributor

Copy link
Member

@mourner mourner 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, pending fix of the Travis build.

});
}
}

template <class T>
void addFeature(const T& multi, const property_map& props) {
void addFeature(const T& multi, const property_map& props, const std::experimental::optional<identifier>& id) {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe alias std::experimental::optional<identifier> to optional_identifier, which is repeated a lot?

@jfirebaugh
Copy link
Contributor Author

Build is failing due to linker errors that stem from this difference:

~/Development/geojson-vt-cpp $ .mason/mason ldflags glfw 3.1.2
-framework Cocoa -framework OpenGL -framework IOKit -framework CoreFoundation -framework CoreVideo
~/Development/geojson-vt-cpp $ .mason/mason ldflags glfw 3.2.1
-framework Cocoa -framework IOKit -framework CoreFoundation -framework CoreVideo

@springmeyer @kkaefer Do you know why -framework OpenGL is not appearing for glfw 3.2.1?

@kkaefer
Copy link
Member

kkaefer commented Feb 7, 2017

@jfirebaugh yeah, I recently debugged exactly that: GLFW only uses higher-level NS* objects, which are part of the Cocoa framework, and does not use any lower-level CGL* symbols. The OpenGL Framework is misleadingly named in that it contains the CGL implementation. The actual GL implementation is in libGL.dylib, which is also linked transitively through CoreVideo.

@jfirebaugh
Copy link
Contributor Author

Ok, what does that mean for fixing the linker error?

@jfirebaugh
Copy link
Contributor Author

@springmeyer Could you look into why the clang-3.8 build is failing?

@springmeyer
Copy link

springmeyer commented Feb 7, 2017

@springmeyer Could you look into why the clang-3.8 build is failing?

Yes. @jfirebaugh latest mason only provides clang++ stable releases of 3.8.1 and 3.9.1 (https://github.com/mapbox/mason/blob/master/CHANGELOG.md#040). So, I'd recommend upgrading to 3.9. This is available in #61

@jfirebaugh jfirebaugh merged commit 2c25bac into master Feb 7, 2017
@jfirebaugh jfirebaugh deleted the fix-59 branch February 7, 2017 23:27
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.

4 participants