Skip to content

Conversation

@bnoordhuis
Copy link
Member

The .bin/ directory in deps/npm/node_modules seens to have been an
accidental check-in in commit e79ccee ("npm: upgrade to v2.1.18").
It causes trouble for distro packagers so delete it and blacklist it.

Fixes: #2839

R=@rvagg? /cc @othiym23

CI: https://ci.nodejs.org/job/node-test-pull-request/359/

@mscdex mscdex added the npm Issues and PRs related to the npm client dependency or the npm registry. label Sep 22, 2015
@rvagg
Copy link
Member

rvagg commented Sep 22, 2015

lgtm

/R=@zkat & @iarna — we just need upstream confirmation that npm isn't relying on .bin in anyway (or is likely to), the symlinks are causing problems when shipped

@thefourtheye
Copy link
Contributor

LGTM, unless it is absolutely necessary to have for npm.

@zkat
Copy link
Contributor

zkat commented Sep 24, 2015

We don't need this atm. I can see us rolling this back in the future if we ever reorganize how our scripts work? But benign enough for now. LGTM

The .bin/ directory in deps/npm/node_modules seens to have been an
accidental check-in in commit e79ccee ("npm: upgrade to v2.1.18").
It causes trouble for distro packagers so delete it and blacklist it.

Fixes: nodejs#2839
PR-URL: nodejs#3004
Reviewed-By: Kat Marchán <kzm@sykosomatic.org>
Reviewed-By: Rod Vagg <r@va.gg>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
@bnoordhuis bnoordhuis closed this Sep 24, 2015
@bnoordhuis bnoordhuis deleted the fix-2839 branch September 24, 2015 18:07
@bnoordhuis bnoordhuis merged commit 6ac79bc into nodejs:master Sep 24, 2015
bnoordhuis added a commit that referenced this pull request Sep 25, 2015
The .bin/ directory in deps/npm/node_modules seens to have been an
accidental check-in in commit e79ccee ("npm: upgrade to v2.1.18").
It causes trouble for distro packagers so delete it and blacklist it.

Fixes: #2839
PR-URL: #3004
Reviewed-By: Kat Marchán <kzm@sykosomatic.org>
Reviewed-By: Rod Vagg <r@va.gg>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
This was referenced Sep 30, 2015
@MylesBorins
Copy link
Contributor

landed in lts-v4.x-staging as 039f73f

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

Labels

npm Issues and PRs related to the npm client dependency or the npm registry.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants