Skip to content

test: explicitly set global in test-repl - #6026

Closed
Trott wants to merge 1 commit into
nodejs:masterfrom
Trott:coffee
Closed

test: explicitly set global in test-repl#6026
Trott wants to merge 1 commit into
nodejs:masterfrom
Trott:coffee

Conversation

@Trott

@TrottTrott commented Apr 3, 2016

Copy link
Copy Markdown
Member

Pull Request check-list

  • Does make -j8 test (UNIX) or vcbuild test nosign (Windows) pass with
    this change (including linting)?
  • Is the commit message formatted according to [CONTRIBUTING.md][0]?

Affected core subsystem(s)

test repl

Description of change

The test intentionally assigns a global. Use global namespace to make it clear that it is intentional and not an accidental leak.

The test intentionally assigns a global. Use `global` namespace to make
it clear that it is intentional and not an accidental leak.
@TrottTrott added repl Issues and PRs related to the REPL subsystem. test Issues and PRs related to the tests. lts-watch-v4.x labels Apr 3, 2016
@bnoordhuis

Copy link
Copy Markdown
Member

LGTM

@benjamingr

Copy link
Copy Markdown
Member

LGTM, though I'm +0 on this. It would be interesting to get the test to run under strict mode though so I guess that's a step in that direction.

@cjihrig

Copy link
Copy Markdown
Contributor

LGTM

1 similar comment
@jasnell

Copy link
Copy Markdown
Member

LGTM

@jasnell

Copy link
Copy Markdown
Member

jasnell pushed a commit that referenced this pull request Apr 4, 2016
The test intentionally assigns a global. Use `global` namespace to make
it clear that it is intentional and not an accidental leak.
PR-URL: #6026
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@jasnell

Copy link
Copy Markdown
Member

Landed in 3df16b1

@jasnelljasnell closed this Apr 4, 2016
MylesBorins pushed a commit that referenced this pull request Apr 5, 2016
The test intentionally assigns a global. Use `global` namespace to make
it clear that it is intentional and not an accidental leak.
PR-URL: #6026
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This was referenced Apr 5, 2016
MylesBorins pushed a commit that referenced this pull request Apr 11, 2016
The test intentionally assigns a global. Use `global` namespace to make
it clear that it is intentional and not an accidental leak.
PR-URL: #6026
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorinsMylesBorins mentioned this pull request Apr 11, 2016
@Trott
Trott deleted the coffee branch January 13, 2022 22:42
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

replIssues and PRs related to the REPL subsystem.testIssues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@Trott@bnoordhuis@benjamingr@cjihrig@jasnell@MylesBorins