Skip to content
This repository was archived by the owner on Dec 15, 2022. It is now read-only.

Add back node-gyp dependency#840

Closed
winstliu wants to merge 4 commits intomasterfrom
wl-add-back-node-gyp
Closed

Add back node-gyp dependency#840
winstliu wants to merge 4 commits intomasterfrom
wl-add-back-node-gyp

Conversation

@winstliu
Copy link
Copy Markdown
Contributor

@winstliu winstliu commented May 4, 2019

Requirements

  • Filling out the template is required. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.
  • All new code requires tests to ensure against regressions

Description of the Change

Reverts most of #832. There's been a few problems with node-gyp being unable to find dependencies or rebuilding failing on 32-bit Windows, which I think may be resolved if we simply go back to depending on it explicitly. I have, however, updated the dependency to match that of npm@6.2.0.

Alternate Designs

#832, which was causing flaky builds on this repo and rebuild errors downstream in atom/atom.

Benefits

Newer node-gyp version (VS2017 support, bugfixes, etc).

Possible Drawbacks

We'll have to remember to update node-gyp each time we update npm.

Verification Process

N/A

Applicable Issues

#832, atom/atom#19189

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants