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

test: explicitly set global in test-repl #6026

Closed
wants to merge 1 commit into from
Closed

Conversation

Trott
Copy link
Member

@Trott Trott commented Apr 3, 2016

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.
@Trott Trott 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
Member

LGTM

@benjamingr
Copy link
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
Contributor

cjihrig commented Apr 3, 2016

LGTM

1 similar comment
@jasnell
Copy link
Member

jasnell commented Apr 4, 2016

LGTM

@jasnell
Copy link
Member

jasnell commented Apr 4, 2016

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
Member

jasnell commented Apr 4, 2016

Landed in 3df16b1

@jasnell jasnell 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>
@MylesBorins MylesBorins mentioned this pull request Apr 11, 2016
@Trott Trott deleted the coffee branch January 13, 2022 22:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
repl Issues and PRs related to the REPL subsystem. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants