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

doc: remove test-npm from general build doc #12840

Closed
wants to merge 1 commit into from

Conversation

@Trott
Copy link
Member

commented May 4, 2017

make test-npm is not particularly robust (currently fails on Linux and
Windows, reportedly) and results in a fair number of requests for help
from people new to the project. It is used when upgrading npm in core,
which only a small number of mostly-predefined people do. Remove it from
the general build doc.

Checklist
Affected core subsystem(s)

doc

doc: remove test-npm from general build doc
`make test-npm` is not particularly robust (currently fails on Linux and
Windows, reportedly) and results in a fair number of requests for help
from people new to the project. It is used when upgrading npm in core,
which only a small number of mostly-predefined people do. Remove it from
the general build doc.

Refs: #12836

@Trott Trott force-pushed the Trott:rm-npm-test branch to f2f182a May 4, 2017

@jasnell
jasnell approved these changes May 5, 2017
@lpinca
lpinca approved these changes May 5, 2017
@cjihrig
cjihrig approved these changes May 5, 2017
@danbev
danbev approved these changes May 5, 2017
@addaleax

This comment has been minimized.

Copy link
Member

commented May 7, 2017

Landed in a398516

@addaleax addaleax closed this May 7, 2017

addaleax added a commit that referenced this pull request May 7, 2017
doc: remove test-npm from general build doc
`make test-npm` is not particularly robust (currently fails on Linux and
Windows, reportedly) and results in a fair number of requests for help
from people new to the project. It is used when upgrading npm in core,
which only a small number of mostly-predefined people do. Remove it from
the general build doc.

Refs: #12836
PR-URL: #12840
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
anchnk pushed a commit to anchnk/node that referenced this pull request May 19, 2017
doc: remove test-npm from general build doc
`make test-npm` is not particularly robust (currently fails on Linux and
Windows, reportedly) and results in a fair number of requests for help
from people new to the project. It is used when upgrading npm in core,
which only a small number of mostly-predefined people do. Remove it from
the general build doc.

Refs: nodejs#12836
PR-URL: nodejs#12840
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@jasnell jasnell referenced this pull request May 11, 2017
@refack

This comment has been minimized.

Copy link
Member

commented May 26, 2017

Not sure if this would have helped with finding out npm/npm#16734, but somewhere we need to regression test npm

@refack

This comment has been minimized.

Copy link
Member

commented May 26, 2017

Ref: #11540

@Trott

This comment has been minimized.

Copy link
Member Author

commented May 27, 2017

Not sure if this would have helped with finding out npm/npm#16734,

@refack No because the test doesn't work on Windows and 16734 is a Windows issue.

This was a documentation-only PR. No tests were removed or anything like that. I think comments on the quality or effectiveness of our npm testing in general should go in a separate issue.

@refack

This comment has been minimized.

Copy link
Member

commented May 27, 2017

I think comments on the quality or effectiveness of our npm testing in general should go in a separate issue.

Ack. My comment wasn't directly related to this PR, definatly not critique, I just needed a thread with context, and I only found #11540 later (mostly had a guilty mind / hindsight is always 20/20 etc.)

@gibfahn gibfahn referenced this pull request Jun 15, 2017
2 of 3 tasks complete
gibfahn added a commit that referenced this pull request Jun 20, 2017
doc: remove test-npm from general build doc
`make test-npm` is not particularly robust (currently fails on Linux and
Windows, reportedly) and results in a fair number of requests for help
from people new to the project. It is used when upgrading npm in core,
which only a small number of mostly-predefined people do. Remove it from
the general build doc.

Refs: #12836
PR-URL: #12840
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins added a commit that referenced this pull request Jul 11, 2017
doc: remove test-npm from general build doc
`make test-npm` is not particularly robust (currently fails on Linux and
Windows, reportedly) and results in a fair number of requests for help
from people new to the project. It is used when upgrading npm in core,
which only a small number of mostly-predefined people do. Remove it from
the general build doc.

Refs: #12836
PR-URL: #12840
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@MylesBorins MylesBorins referenced this pull request Jul 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
10 participants
You can’t perform that action at this time.