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 types PointZ, PointM and PointZM #56

Merged
merged 6 commits into from
Dec 31, 2016
Merged

Add types PointZ, PointM and PointZM #56

merged 6 commits into from
Dec 31, 2016

Conversation

Kabie
Copy link

@Kabie Kabie commented Dec 29, 2016

I'm not sure I did it right on type = Utils.hex_to_type(type &&& 0xdf_ff_ff_ff).

@bryanjos
Copy link
Contributor

@Kabie looks good! For you concern on type = Utils.hex_to_type(type &&& 0xdf_ff_ff_ff), I'm guessing it's fine since all the tests pass. This project was one of my first in Elixir so there are probably a lot of things specifically in that area that could have been done better.

If all is good I can merge it. Saw the "WIP" earlier and didn't know if you were finished or not

@bryanjos
Copy link
Contributor

I learned something from this PR. I thought that making the coordinate values tuples would allow them to be used for more than just 2D coordinates. I never encountered Z, M, or ZM before and now I see why they have to be different types because could be some ambiguity between Z and M geometries.

@Kabie
Copy link
Author

Kabie commented Dec 30, 2016

Thanks. My first thought was to reuse Geo.Point but not succeed.

I think it's finished now, feel free to merge it.

@bryanjos bryanjos merged commit 25593d9 into felt:master Dec 31, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants