search

mapbox / mapnik-omnivore

Node module that returns metadata about spatial files.
44 stars 19 forks source link

Upgrade dependencies #180

Closed perliedman closed 4 years ago

perliedman commented 5 years ago

Since the latest released version is very hard to build on a modern Ubuntu machine (at least for me!), I have updated the dependencies to modern versions. Most importantly, mapnik is updated to 4.2.1.

I also accepted all of npm audit's suggested fixes, even for possibly breaking changes. This in turn made be replace jscs with eslint.

Happy to break this PR into separate pieces if that makes more sense.

perliedman commented 5 years ago

BTW, the tests break in Node 4, since it isn't compatible with eslint. Don't know if Node 4 is still a priority to the project (Node 4 has reached end of life). As mentioned above, I am happy to split of the eslint parts to a separate PR if this is an issue for you.

perliedman commented 5 years ago

Hi @mourner! :wave: Great feedback as always!

I have perfomed the changes, just let me know if there is something more to be done to get this merged. For example, should I get rid of the Node 4 builds in .travis.yml and maybe add Node 8 and 10 instead?

mourner commented 5 years ago

Oh, sure! Node 4 reached End of Life (no further updates even if critical) a year ago, and Node 6 reaches it next month. So I think we should switch to versions 8 and “node” (for latest). @mapsam what do you think?

perliedman commented 5 years ago

Appears there are no pre-built binaries for gdal 0.9.8 and Node 11, so Travis will compile gdal which might take some time (but hopefully work :grimacing: ).

perliedman commented 5 years ago

Update: Travis failed to build gdal for Node 11. I guess we could fix the env to allow this, but for now I just use Node 10, which has prebuilt gdal binaries. Hope that is ok.

mourner commented 5 years ago

I think it’s fine! Node 11 is not LTS anyway.

mapsam commented 5 years ago

So I think we should switch to versions 8 and “node” (for latest)

Agree! Thanks @perliedman + @mourner 😄

perliedman commented 5 years ago

So good to merge?

Also, if it is not too much trouble, I have similar PRs, but a bit simpler, open for tilelive-mapnik (https://github.com/mapbox/tilelive-mapnik/pull/124) and tilelive-bridge (https://github.com/mapbox/tilelive-bridge/pull/103).

mapsam commented 4 years ago

Sorry @perliedman - I assumed @mourner was taking the merge/release on but should have checked with him first. Merging now!