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

chore: re-enable no-undef rule for tests #974

Merged
merged 2 commits into from Mar 12, 2024
Merged

Conversation

boneskull
Copy link
Contributor

@boneskull boneskull commented Jan 30, 2024

This needs #960 to pass CI, but I cannot target a branch in a fork.

no-undef was disabled because we use a lot of undefined variables, but it's better to create lint exceptions on a line-by-line or file-by-file basis, since it can cause the problem fixed in #960.

@boneskull boneskull requested a review from a team as a code owner January 30, 2024 21:13
@github-actions github-actions bot added pkg:lavamoat-core Changes in package lavamoat-core pkg:lavamoat-browserify Changes in package lavamoat-browserify pkg:lavamoat-tofu Changes in package lavamoat-tofu pkg:@lavamoat/lavapack Changes in package @lavamoat/lavapack labels Jan 30, 2024
@@ -1,3 +1,4 @@
/* eslint-disable no-undef */
Copy link
Contributor

@legobeat legobeat Jan 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than entirely disabling the rule here, how about listing the globals actually used?

Suggested change
/* eslint-disable no-undef */
/* global checkGlobal, checkSelf, checkThis, checkWindow */

Copy link
Contributor Author

@boneskull boneskull Mar 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ahh, needs /* global someVar: write */: and then another no-undef disable above it 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is what made it work:

// eslint-disable-next-line no-unused-vars
/* global checkThis: true, checkSelf: true, checkWindow: true, checkGlobal: true */

@github-actions github-actions bot added the pkg:@lavamoat/webpack Changes in package @lavamoat/webpack label Mar 8, 2024
@boneskull
Copy link
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @boneskull and the rest of your teammates on Graphite Graphite

@naugtur
Copy link
Member

naugtur commented Mar 12, 2024

I'm skeptical (as in, all undef warnings will be ignored anyway as we move forward, just with added manual labor)
But test should not be modified too often, so I don't mind.

@boneskull boneskull merged commit ce9e655 into main Mar 12, 2024
25 checks passed
@boneskull boneskull deleted the boneskull/no-undef branch March 12, 2024 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg:@lavamoat/lavapack Changes in package @lavamoat/lavapack pkg:@lavamoat/webpack Changes in package @lavamoat/webpack pkg:lavamoat-browserify Changes in package lavamoat-browserify pkg:lavamoat-core Changes in package lavamoat-core pkg:lavamoat-tofu Changes in package lavamoat-tofu
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants