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

[WIP] Add repo-wide ESLint config #781

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

[WIP] Add repo-wide ESLint config #781

wants to merge 2 commits into from

Conversation

six-shopify
Copy link
Contributor

@six-shopify six-shopify commented Feb 12, 2025

What are you adding in this PR?

theme-tools seems to only have ESLint configured for the vscode-extension package. For better or worse, ESLint does catch a lot of legitimate issues. (Though as a side note I find it annoying that many configs, including Shopify's shared config, scream at MAXIMUM ERROR LEVEL for things that are more akin to stylistic nits — IMO it's better to differentiate between nits and errors but to still enforce consistent style in CI with --max-warnings=0.)

Anyway, I added a quick global ESLint config and squelched a bunch of the benign errors in rule overrides. It'll take further work to get everything green so we can start running this in CI, but it's already pointing out some things that look like possible errors. I'll leave them untouched for now, but it might be nice to start surfacing these lints to contributors, even just in the editor.

This is WIP because I aligned the vscode-extension config with the global config, but that'll cause its pretest script to fail right now.

@six-shopify six-shopify requested a review from albchu February 12, 2025 20:37
Base automatically changed from add-dev-yml to main February 13, 2025 23:17
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.

1 participant