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

Enable linting and formatting in examples #1981

Closed
1 task
maribethb opened this issue Sep 26, 2023 · 3 comments · Fixed by #2182
Closed
1 task

Enable linting and formatting in examples #1981

maribethb opened this issue Sep 26, 2023 · 3 comments · Fixed by #2182
Assignees
Labels
type: feature request New feature or request

Comments

@maribethb
Copy link
Contributor

Check for duplicates

  • I have searched for similar issues before opening a new one.

Component

all examples

Problem

Current behavior: examples are not linted, nothing is formatted
After #1946: examples are excluded from linting and formatting

We want to lint and format examples as well

Request

  1. Remove examples from the ignore list in eslint.config.js
  2. Remove examples from the ignore list in .prettierignore
  3. Potentially need to add new files to the ignore list(s) if there are any files in the examples that shouldn't be formatted. Commit all changes so far
  4. Run npm run lint:fix and commit the changes.
  5. Run npm run format and commit the changes.
  6. Run npm run lint, manually fix any remaining lint errors or warnings, and commit the changes.

It is important that steps 4, 5, and 6 each be committed separately, as this will make reviewing and rebasing the PR easier.

Note that some examples specify their own lint command and I'm not sure how this will interact with the new flat config. If you attempt this process and find issues with linting any of the examples, let us know in the comments!

Alternatives considered

No response

Additional context

No response

@alicialics
Copy link
Contributor

alicialics commented Oct 27, 2023

There were 1k+ eslint errors, but most of them were caused by a few select plugins (eg interpreter-demo). Ignoring them brings down the number to 125 errors (and many more warnings).

It might be simpler to do it example by example to make it easier to review since not all of changes are trivial and its hard to guarantee a change is correct without actually running it and hopefully test what you've changed.

We can use the re-include pattern to ignore all examples but a project you are working on
https://eslint.org/docs/latest/use/configure/ignore

Lines preceded by ! are negated patterns that re-include a pattern that was ignored by an earlier pattern.

@BeksOmega
Copy link
Contributor

Believe this was closed by #2070, correct me if I'm wrong @alicialics !

@alicialics
Copy link
Contributor

I think there are some work left over. #2068 runs prettier on all example files as well as enabling eslint on some examples. After #2068 is merged I think there are significant work in enabling eslint on the rest of examples.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature request New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants