Skip to content

Conversation

gbroccolo
Copy link
Collaborator

The following PR takes care of adding the support for the following Extended geometries for point type: PointZ (3D), PointM (2D+1D), PointZM (3D+1D), PointS (2D + SRID info), PointZS (3D + SRID info), PointMS (2D+1D + SRID info), PointZMS (3D+1D + SRID info). This is related to issue #16 . Still need to add support for all the other extended geometries (i.e. the same work needed for linestring, etc.).

@coveralls
Copy link

coveralls commented May 17, 2020

Pull Request Test Coverage Report for Build 489

  • 126 of 126 (100.0%) changed or added relevant lines in 7 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.5%) to 55.463%

Totals Coverage Status
Change from base Build 487: 0.5%
Covered Lines: 6254
Relevant Lines: 11276

💛 - Coveralls

@ARolek ARolek requested a review from gdey May 18, 2020 03:31
Copy link
Member

@gdey gdey left a comment

Choose a reason for hiding this comment

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

This looks really good. I think we should move forward in this direction. Only thing I saw, was to keep the name of the testcase array consistent with tests. Overall this look great!

Thanks

align naming of test cases variable for point types to the rest of tests modules
@gbroccolo gbroccolo force-pushed the add-extended-geoms branch from 23b8b48 to 7912f32 Compare May 23, 2020 18:34
@gbroccolo gbroccolo requested a review from gdey May 23, 2020 18:38
@gdey
Copy link
Member

gdey commented May 26, 2020

@gbroccolo I'm assuming you are going to be adding the other types to this pull request, as well.

@ARolek
Copy link
Member

ARolek commented May 26, 2020

@gdey

I'm assuming you are going to be adding the other types to this pull request, as well.

Which additional types are you referring to?

@gbroccolo
Copy link
Collaborator Author

@gbroccolo I'm assuming you are going to be adding the other types to this pull request, as well.

Yes @gdey, I can continue to push changes to this PR. Let me change the title then (it refers just to Point data type) and change it to be a Draft more then a PR.

@ARolek the additional types are the extentions for Linestring, Polygons, etc.

@gbroccolo gbroccolo marked this pull request as draft May 27, 2020 09:26
@gbroccolo gbroccolo changed the title Add extended points types Add extended geometry types as expected by EWKB standard May 27, 2020
@ARolek
Copy link
Member

ARolek commented May 27, 2020

@gdey we could merge in the extended point types now and then @gbroccolo could work on extending the other types. Having extended point types is already a win and a logic commit. I don't want this PR to get too big or blocked by additional enhancements. Thoughts?

@gbroccolo thanks again for tackling this!

@gdey
Copy link
Member

gdey commented May 27, 2020

I'm good with merging this.
@gbroccolo you will need to remove the draft tag on the pull request.

@gbroccolo gbroccolo marked this pull request as ready for review May 27, 2020 20:40
@gbroccolo gbroccolo changed the title Add extended geometry types as expected by EWKB standard Add extended geometry types for Point as expected by EWKB standard May 27, 2020
@gbroccolo
Copy link
Collaborator Author

@gdey @ARolek I changed the status from draft to ready for review.

Copy link
Member

@gdey gdey left a comment

Choose a reason for hiding this comment

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

LGTM

@gdey gdey merged commit 8b3e487 into go-spatial:master May 29, 2020
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