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

feat(package): use node-postal module compatible with node12 #245

Merged
merged 1 commit into from
Mar 13, 2020

Conversation

orangejulius
Copy link
Member

@orangejulius orangejulius commented Mar 13, 2020

Extracted from other recent PRs. This allows us to support actually running the interpolation service on Node.js 12

If we end up merging something like #146 which unbundles the libpostal service from this repo, then we won't not need this any more, but it's a small enough change that it's worth it.

Connects pelias/pelias#800

orangejulius added a commit to pelias/docker-libpostal_baseimage that referenced this pull request Mar 13, 2020
The libpostal baseimage has historically stayed on the Node.js 10 branch
of the Pelias base image due to incompatibility between [node-postal and
Node.js 12](openvenues/node-postal#24).

We are now using a Node.js 12 compatible fork of the `node-postal`
libary, so we can now bring the libpostal baseimage back to parity with
the other Docker images.

This will reduce complexity, maintenance times, and download sizes for
Pelias containers.

Connects pelias/interpolation#245
Connects pelias/pelias#800
orangejulius added a commit to pelias/docker-libpostal_baseimage that referenced this pull request Mar 13, 2020
The libpostal baseimage has historically stayed on the Node.js 10 branch
of the Pelias base image due to incompatibility between [node-postal and
Node.js 12](openvenues/node-postal#24).

We are now using a Node.js 12 compatible fork of the `node-postal`
libary, so we can now bring the libpostal baseimage back to parity with
the other Docker images.

This will reduce complexity, maintenance times, and download sizes for
Pelias containers.

Connects pelias/interpolation#245
Connects pelias/pelias#800
@missinglink
Copy link
Member

missinglink commented Mar 13, 2020

Oh, I thought it was either 10 or 12.

Build matrix shows this works with both, so I'm 👍 👍.

Although I think linking to a master branch could mean unexpected changes in the future, maybe we can either fork this code to something we control or pin a specific commit?

@orangejulius
Copy link
Member Author

Good call, I'll pin to the latest commit.

This updates our dependencies to use the Node.js 12 (and 10) compatible
fork of [node-postal](https://github.com/openvenues/node-postal).

The fork is located at [imothee/node-postal](https://github.com/imothee/node-postal).

Connects pelias/pelias#800
@orangejulius orangejulius merged commit a121a6a into master Mar 13, 2020
@orangejulius orangejulius deleted the node-postal-12 branch March 13, 2020 19:32
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.

2 participants