Skip to content

Conversation

@cpeel
Copy link
Member

@cpeel cpeel commented Dec 25, 2025

Not sure how we missed having all the JS files in eslint, but this sweeps the codebase to add them all.

No sandbox, the CI passing is all we need.

@cpeel cpeel requested review from chrismiceli and srjfoo December 25, 2025 01:36
@cpeel cpeel self-assigned this Dec 25, 2025
"scripts/api.js",
"scripts/character_test.js",
"scripts/control_bar.js",
"scripts/dropdown.js",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I can't remember why we didn't do *.js, because we should be able to, with appropriate excludes if necessary.

Copy link
Member Author

Choose a reason for hiding this comment

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

You didn't, this is me being dumb. We don't need this PR.

If I look closely this is the modules section which should only have the modules in it -- not the scripts which are covered under the wildcard above. And that's why the other changes were needed -- because they are being checked as modules even though they weren't.

Sorry for the kerfuffle and thanks for calling this out Chris.

/* exported barLineGraph, stackedAreaGraph, pieGraph */

// eslint-disable-next-line no-unused-vars
const { barLineGraph, stackedAreaGraph, pieGraph } = (function () {
Copy link
Collaborator

Choose a reason for hiding this comment

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

interesting the exported from above doesn't resolve this.

@@ -1,4 +1,4 @@
/*global */
/* eslint-disable no-unused-vars */
Copy link
Collaborator

Choose a reason for hiding this comment

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

hmm, I wonder if more exported are needed? Since we switched to modules though, we should rely on exports or window property assignments to resolve this?

@cpeel cpeel closed this Dec 25, 2025
@cpeel cpeel deleted the add-missing-js-to-eslint branch December 25, 2025 16:02
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.

3 participants