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

Bug - Building with node 10 #484

Merged
merged 1 commit into from
Nov 19, 2018
Merged

Bug - Building with node 10 #484

merged 1 commit into from
Nov 19, 2018

Conversation

tgandrews
Copy link
Contributor

What is the context of this PR?

Upgrade firebase which had the dependency on the old version of grpc which was failing to build on node 10.

How to review

  1. Ensure CI passes
  2. Ensure it works locally with node 10.

@tgandrews
Copy link
Contributor Author

Snyk is complaining about a transient dependency having a security vulnerability. But AFAICT they are not going to resolve it isaacs/chownr#14

So I think we should ignore this in synk.

Copy link
Contributor

@jonnyshaw89 jonnyshaw89 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works for me now.

@tgandrews
Copy link
Contributor Author

@tgandrews tgandrews force-pushed the bug-building-node-10 branch 2 times, most recently from cc12ae5 to ea15739 Compare November 19, 2018 09:17
@SamGodwin2
Copy link
Contributor

Is it worth updating the .nvmrc file to lts/dubnium?

Copy link
Contributor

@samiwel samiwel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Agree with @SamGodwin2's comment, but may as well make that change on the monorepo now?

Edit: actually could make the change here. It will be pulled down into the monorepo then.

@tgandrews tgandrews force-pushed the bug-building-node-10 branch from ea15739 to 0b7312e Compare November 19, 2018 10:58
- Upgrade firebase as it had a dependency on an old version of grpc
- Update travis to use node 10
- Update nvmrc to node 10 lts
- Update docker to node 10 lts
@tgandrews tgandrews force-pushed the bug-building-node-10 branch from 0b7312e to d87b33f Compare November 19, 2018 11:00
@tgandrews
Copy link
Contributor Author

@samiwel @SamGodwin2 I updated the nvmrc and Dockerfile.

Copy link
Contributor

@SamGodwin2 SamGodwin2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All works for me. 👍

@tgandrews tgandrews merged commit c4536c8 into master Nov 19, 2018
@tgandrews tgandrews deleted the bug-building-node-10 branch November 19, 2018 11:48
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.

4 participants