search

avernois / devfriendlyplaces

Note: devfriendlyplaces has moved to an organisation. Code and data can be found at https://github.com/devfriendlyplaces
61 stars 60 forks source link

Use of geojson #191

Closed francois2metz closed 7 years ago

francois2metz commented 7 years ago

Ahem. So @egaillot and I, we refactored as an exercise all places files to use the fantastic geojson standard instead of the custom json format you had. You may want to use it.

It brings some benefits:

Awesome isn't it?

We did some improvements to the test suite in order to run them easily (without phantom).

francois2metz commented 7 years ago

I created 2 separate branches to split this one if you want:

francois2metz commented 7 years ago

I opened 2 PRs #192 and #193 to cleanup this one. While it's not merged, the correct commit range is: https://github.com/avernois/devfriendlyplaces/pull/191/files/d8996daed5beac2ccc073427ecc3a1e5c1e65ee7..dcea3542afacdf2b0191843ee181b6474157f13f#diff-3c9aeac206d50f871576ea9d38da8dfa

avernois commented 7 years ago

Hello,

sorry for the late answer. This looks awesome, I was thinking of that move to GeoJSON for quite a while. I'll check the code this morning and merge if everything looks fine.

avernois commented 7 years ago

Well, when I look at the resulting json, I remember why I did not do it earlier :) I wonder if it's not too complex to be written "by hand". With the current (I think easy) flat format, most of PR I receive have errors. Most of them are detected with 'jq' (and we could probably enforce it by creating a schema to check files against)

I like the idea of using a "standard" format, but I'm not sure that it worth the induce complexity.

I really don"t know what to do on that one. I'm open for comments/advice/opinion/suggestion :)

francois2metz commented 7 years ago

I agree with the format :)

One solution it to keep the current one, then pre-compile the site to transform the original json to geojson on deploy.

Another solution I found is to use http://geojson.io.