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: specify global object for globals #36498

Merged
merged 1 commit into from
Dec 16, 2020
Merged

Conversation

Trott
Copy link
Member

@Trott Trott commented Dec 13, 2020

Be explicit about using global.externalizeString() etc. in
test-fs-write instead of disabling the no-undef ESLint rule.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Dec 13, 2020
@Trott Trott added request-ci Add this label to start a Jenkins CI on a PR. and removed test Issues and PRs related to the tests. labels Dec 13, 2020
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 13, 2020
@nodejs-github-bot
Copy link
Collaborator

@Trott
Copy link
Member Author

Trott commented Dec 13, 2020

An additional change that might be good here is to add a comment explaining what externalizeString() and isOneByteString() do, as I believe these are the only places they appear in the Node.js code base (other than vendored-in code like the V8 source code).

@Trott
Copy link
Member Author

Trott commented Dec 13, 2020

An additional change that might be good here is to add a comment explaining what externalizeString() and isOneByteString() do, as I believe these are the only places they appear in the Node.js code base (other than vendored-in code like the V8 source code).

I could try to write something vague and hand-wave-y about string encoding and garbage collection, but I'd much rather have someone who knows what they're talking about it explain it. (As an added bonus, then I might understand it!)

@nodejs-github-bot
Copy link
Collaborator

Be explicit about using `global.externalizeString()` etc. in
test-fs-write instead of disabling the `no-undef` ESLint rule.

PR-URL: nodejs#36498
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@Trott
Copy link
Member Author

Trott commented Dec 16, 2020

Landed in 44243e5

@Trott Trott merged commit 44243e5 into nodejs:master Dec 16, 2020
@Trott Trott deleted the no-undef-fs branch December 16, 2020 13:22
targos pushed a commit that referenced this pull request Dec 21, 2020
Be explicit about using `global.externalizeString()` etc. in
test-fs-write instead of disabling the `no-undef` ESLint rule.

PR-URL: #36498
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: James M Snell <[email protected]>
targos pushed a commit that referenced this pull request May 1, 2021
Be explicit about using `global.externalizeString()` etc. in
test-fs-write instead of disabling the `no-undef` ESLint rule.

PR-URL: #36498
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@danielleadams danielleadams mentioned this pull request May 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants