search

NetTopologySuite / NetTopologySuite.IO.SpatiaLite

SpatialLite IO module for NTS.
BSD 3-Clause "New" or "Revised" License
7 stars 9 forks source link

added GeoPackage support #3

Closed DGuidi closed 5 years ago

CLAassistant commented 5 years ago

CLA assistant check
All committers have signed the CLA.

DGuidi commented 5 years ago

thanks a lot @airbreather for the detailed review, hope to fix in next few days. I've added some comments to your reviews, for thinks that are not clear to me.

airbreather commented 5 years ago

thanks a lot @airbreather for the detailed review, hope to fix in next few days. I've added some comments to your reviews, for thinks that are not clear to me.

No problem, I'll respond inline.

Just FYI, I noticed you reacted to every comment that GitHub shows by default, but none of the ones that GitHub hides by default, so I just wanted to make sure you're aware of this button: image

DGuidi commented 5 years ago

I've fixed some comments, I hope to fix all other stuff tomorrow

DGuidi commented 5 years ago

@airbreather need another check, see open comments. please take a brief look also to nuspec files. thanks a lot.

added two more tests to show how srid is handled in GeoPackageReader. to me looks that HandleSRID should be true by default, and possibly always true (i.e: not accepting a false as value): it's just an int readed in any case in header, and I need to explicitely exclude from readed values just because HandleSRID is false...

DGuidi commented 5 years ago

ok looks we're ready to merge

airbreather commented 5 years ago

One last thing... test project doesn't add the SolutionDir property, so it may not build properly from the command line (which is how Travis-CI runs it) after changing that one to use $(NTSPackageReferenceVersion).

DGuidi commented 5 years ago

fixed solutiondir

DGuidi commented 5 years ago

should I "push the button" to merge? Execute