test,tools: limit lint tolerance of gc global#6324
test,tools: limit lint tolerance of gc global#6324Trott wants to merge 2 commits intonodejs:masterfrom
Conversation
|
LGTM, I guess. Would this allow |
It looks like it, based on the change to LGTM |
|
This is fine and the change LGTM but I'm curious.. why? |
7da4fd4 to
c7066fb
Compare
|
@jasnell wrote:
I prefer exceptions to lint rules to be applied only as narrowly as necessary. So rather than applying an exception to the 1324 JS files in |
|
@bnoordhuis asked:
Yes, that would pass linting. |
|
LGTM |
|
@Trott ... ok :-) thanks! |
|
Anyone prefer |
|
Dropping the eslint directive is preferably IMO in case things change in the future on their side or ours. |
Lint rules permitted the `gc` global in any test file. This change limits it to just the files that need it.
|
Removed the directives and replaced instances of |
|
LGTM |
1 similar comment
|
LGTM |
|
CI is green except for unrelated and known-problematic tick processor test on OS X which will be fixed Very Soon. |
Lint rules permitted the `gc` global in any test file. This change limits it to just the files that need it. PR-URL: nodejs#6324 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
|
Landed in 6c49714. |
Lint rules permitted the `gc` global in any test file. This change limits it to just the files that need it. PR-URL: #6324 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
|
not landing cleanly @Trott feel free to tag don't land if you don't want to backport |
|
@thealphanerd @Trott I think this can land on v4.x once #6871 lands. |
Checklist
Affected core subsystem(s)
test tools
Description of change
Lint rules permitted the
gcglobal in any test file. This changelimits it to just the files that need it.