search

mapbox / geojson-merge

Merge multiple GeoJSON files into one FeatureCollection.
ISC License
233 stars 33 forks source link

Fix example of geojsonMerge.merge in README #27

Closed brendannee closed 6 years ago

andrewharvey commented 6 years ago

oh I see the problem now.

That section of the README is auto-generated with yarn run doc, but it was actually an issue in the source. So a one line change there https://github.com/mapbox/geojson-merge/pull/26/commits/e0040e48bf6ae45a1539a6bf630b507a6197744b#diff-168726dbe96b3ce427e7fedce31bb0bc and it's all good. To save the trouble I've included the full fix in my maintenance PR at #26.

Thanks for noticing this and taking the effort to open the PR.

brendannee commented 6 years ago

In the example, I think its confusing to use the variable name mergedStream since the merge method is distinct from mergeFeatureCollectionStream. Also, the last line mergedStream.pipe(process.stdout); may also be confusing as part of the example.

andrewharvey commented 6 years ago

Yeah good point! I've fixed this in my PR, it looks like https://github.com/andrewharvey/geojson-merge/blob/ff9c7245bdaae8a2c5f0e5a62f475f5ed7d133d5/README.md

I'll ping Mapbox to try to get it merged.

brendannee commented 6 years ago

Looks good!