Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use @osmcha/osmchange-parser and @osmcha/osm-changeset-xml-parser; remove lib/xml.js #21

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jake-low
Copy link
Contributor

I wrote two new helper modules recently, osmchange-parser and osm-changeset-xml-parser. They both use saxjs, so they work in the browser and in Node, and like v2.0.0 of osm-adiff-parser they are written as ES6 modules and transpiled to CommonJS at publish time, so you can require or import them, whichever your project uses. They also both have unit tests. Code review on these repos is totally welcome if you're inclined. 🙂

Anyway, this PR pulls in those two modules as new dependencies and removes lib/xml.js which implemented similar functionality. It also drops the htmlparser2 dependency which is no longer required.

I tested carefully to make sure that the osm-adiff-service produces identical output before and after this change. The new function makeBackwardsCompatible in lib/get-changesets.js does some type conversion and other reshaping of the output data in order to ensure this. We might want to think about creating a formal spec for the real-changeset schema in the future, but for now I'm assuming any change might break existing consumers and should therefore be avoided.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant