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

[now dev] Install builders with yarn instead of npm #2272

Merged
merged 7 commits into from Apr 30, 2019

Conversation

TooTallNate
Copy link
Member

@TooTallNate TooTallNate commented Apr 29, 2019

Depends on #2270.

@TooTallNate TooTallNate requested review from styfle and leo April 29, 2019 21:25
Also invokes via `process.execPath` so that the pkg'd node is used
instead of a global one.

Depends on #2270.
@TooTallNate TooTallNate force-pushed the update/now-dev-use-yarn-to-install-builders branch from 30d3726 to 7188502 Compare April 30, 2019 00:45
Not the `node_modules/.bin` dir, because `yarn` cleans up that directory
when installing modules, so it deletes itself.
@TooTallNate TooTallNate requested a review from styfle April 30, 2019 02:23
Co-Authored-By: TooTallNate <n@n8.io>
@codecov-io
Copy link

codecov-io commented Apr 30, 2019

Codecov Report

Merging #2272 into canary will increase coverage by 0.24%.
The diff coverage is 83.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           canary    #2272      +/-   ##
==========================================
+ Coverage   10.35%   10.59%   +0.24%     
==========================================
  Files         246      246              
  Lines        9032     9023       -9     
  Branches      987      986       -1     
==========================================
+ Hits          935      956      +21     
+ Misses       7986     7953      -33     
- Partials      111      114       +3
Impacted Files Coverage Δ
src/commands/dev/lib/yarn-installer.ts 81.63% <100%> (+47.73%) ⬆️
src/commands/dev/lib/builder-cache.ts 67.27% <100%> (+0.3%) ⬆️
src/commands/dev/lib/dev-server.ts 40.27% <66.66%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 37964c1...5411ea0. Read the comment docs.

Copy link
Contributor

@thasophearak thasophearak left a comment

Choose a reason for hiding this comment

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

LGTM 🙌🙏

@TooTallNate TooTallNate merged commit 5ec4545 into canary Apr 30, 2019
@TooTallNate TooTallNate deleted the update/now-dev-use-yarn-to-install-builders branch April 30, 2019 02:58
leo pushed a commit that referenced this pull request Apr 30, 2019
* [now dev] Install builders with `yarn` instead of `npm`

Also invokes via `process.execPath` so that the pkg'd node is used
instead of a global one.

Depends on #2270.

* Install `yarn` to the builder cache dir

Not the `node_modules/.bin` dir, because `yarn` cleans up that directory
when installing modules, so it deletes itself.

* Remove unnecessary unit test

* Always install `yarn`

* Pass in the `yarnPath` to `installBuilders()`

* Restore unit test

* Remove unused `delimiter` import

Co-Authored-By: TooTallNate <n@n8.io>
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.

None yet

4 participants