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

test: refactor parallel/test-process-env.js #8324

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
4 participants
@Trott
Member

Trott commented Aug 29, 2016

Checklist
  • make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

test process

Description of change
  • Remove disabling of lint rule
  • Enable commented-out test
  • == -> ===
  • var -> const
@jasnell

This comment has been minimized.

Show comment
Hide comment
@jasnell

jasnell Aug 29, 2016

Member

LGTM

Member

jasnell commented Aug 29, 2016

LGTM

@cjihrig

This comment has been minimized.

Show comment
Hide comment
@cjihrig

cjihrig Aug 29, 2016

Contributor

LGTM

Contributor

cjihrig commented Aug 29, 2016

LGTM

@Trott

This comment has been minimized.

Show comment
Hide comment
@Trott

Trott Aug 31, 2016

Member

CI, although Raspberry Pi issues abound at the moment. Will almost certainly need to re-run. But just to suss out anything that might be lurking elsewhere. CI: https://ci.nodejs.org/job/node-test-pull-request/3898/

Member

Trott commented Aug 31, 2016

CI, although Raspberry Pi issues abound at the moment. Will almost certainly need to re-run. But just to suss out anything that might be lurking elsewhere. CI: https://ci.nodejs.org/job/node-test-pull-request/3898/

@Trott

This comment has been minimized.

Show comment
Hide comment
@Trott

Trott Aug 31, 2016

Member

Looks like the test that was commented out fails on AIX and Windows. I'll comment it out again. It may make sense to skip it only on those platforms, but I'd want something like that to happen in a separate PR, I think.

Member

Trott commented Aug 31, 2016

Looks like the test that was commented out fails on AIX and Windows. I'll comment it out again. It may make sense to skip it only on those platforms, but I'd want something like that to happen in a separate PR, I think.

test: refactor parallel/test-process-env.js
* Remove disabling of lint rule
* Enable commented-out test
* == -> ===
* var -> const
@Trott

This comment has been minimized.

Show comment
Hide comment
@Trott

This comment has been minimized.

Show comment
Hide comment
@Trott

Trott Aug 31, 2016

Member

CI passed. (Yellow because of AIX perma-yellow right now.)

Member

Trott commented Aug 31, 2016

CI passed. (Yellow because of AIX perma-yellow right now.)

Trott added a commit to Trott/io.js that referenced this pull request Aug 31, 2016

test: refactor parallel/test-process-env.js
* Remove disabling of lint rule
* == -> ===
* var -> const

PR-URL: nodejs#8324
Reviewed-By: jasnell - James M Snell <jasnell@gmail.com>
Reviewed-By: cjihrig - Colin Ihrig <cjihrig@gmail.com>
@Trott

This comment has been minimized.

Show comment
Hide comment
@Trott

Trott Aug 31, 2016

Member

Landed in 266270e

Member

Trott commented Aug 31, 2016

Landed in 266270e

@Trott Trott closed this Aug 31, 2016

@Fishrock123 Fishrock123 referenced this pull request Sep 6, 2016

Closed

v6.6.0 pre-proposal #8428

Fishrock123 added a commit to Fishrock123/node that referenced this pull request Sep 8, 2016

test: refactor parallel/test-process-env.js
* Remove disabling of lint rule
* == -> ===
* var -> const

PR-URL: nodejs#8324
Reviewed-By: jasnell - James M Snell <jasnell@gmail.com>
Reviewed-By: cjihrig - Colin Ihrig <cjihrig@gmail.com>

Fishrock123 added a commit that referenced this pull request Sep 9, 2016

test: refactor parallel/test-process-env.js
* Remove disabling of lint rule
* == -> ===
* var -> const

PR-URL: #8324
Reviewed-By: jasnell - James M Snell <jasnell@gmail.com>
Reviewed-By: cjihrig - Colin Ihrig <cjihrig@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment