search

georust / geojson

Library for serializing the GeoJSON vector GIS file format
https://crates.io/crates/geojson
Apache License 2.0
276 stars 60 forks source link

Display GeoJSON string for geojson::Value #151

Closed pjsier closed 3 years ago

pjsier commented 3 years ago

Closes #149. Uses the existing Display implementation on Geometry to display the GeoJSON string for geojson::Value.

The issue mentions needing to wrap the Value in both a Geometry and GeoJson, but I'm only wrapping in a Geometry here so let me know if I missed anything. I'm also not sure if there's a clean way of avoiding the clone() when creating the Geometry. Happy to make any changes, thanks!

pjsier commented 3 years ago

Also noticed that this didn't trigger CI because it's only set to run on push. Is updating that to run on pull requests something that would make sense for another issue?

michaelkirk commented 3 years ago

Also noticed that this didn't trigger CI because it's only set to run on push. Is updating that to run on pull requests something that would make sense for another issue?

I haven't ever actually tested this theory or read more than cursorily, but my understanding is by running CI automatically when anyone runs a PR, we're effectively giving everyone with a GH account write access to our repository, since they can edit the workflow file in the PR (and the actions runner has a read/write GH token).

So, mostly for that reason I prefer the current workflow for this project and most of my public projects, but if there's a broad preference for an alternative (or if I'm misunderstanding), I'd be open to discussing it.

Since you already have write access to this repository, you could push up your feature branch to georust/geojson rather than your fork and CI will run automatically.

People who aren't in the georust/core cannot, and so need someone with bors access for this repository to manually trigger their CI, after, presumably verifying that the PR is sane:

bors try

bors[bot] commented 3 years ago

try

Build succeeded:

pjsier commented 3 years ago

Thanks for clarifying! In my understanding the GitHub token included in a fork only has read access to the source repo, so even if someone modifies the workflow in a malicious fork they aren't able to modify the source. I'm basing that on some posts in the GitHub support forum.

I made the PR from a fork out of habit here, but if we want to stick with this workflow that's fine with me

frewsxcv commented 3 years ago

bors r+

bors[bot] commented 3 years ago

Build succeeded: