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

fix(linting): fix local npm lint scripts #1207

Merged
merged 1 commit into from
Jun 20, 2024

Conversation

jpetto
Copy link
Collaborator

@jpetto jpetto commented Jun 18, 2024

Goal

fix local npm lint scripts

  • also run lint-fix and add the fixed files

Test Locally

pull down the latest main and run npm run lint-check. for me, this reported one error in a file in node_modules - which is a directory it shouldn't even check. it also does not report the errors that my fix here does.

still on the main branch, update the package.json lint scripts to match the ones in this PR and run them. you should get the correct output (like 80 files needing fixing).

Implementation Decisions

does anyone know why we were running tsc before the lint scripts? why would we need to compile to only lint .ts(x) files?

also - unclear why the files caught here are no being caught by the github linter. other repositories are experiencing similar local/github linting discrepancies - wonder if some downstream package got changed...

- also run lint-fix and add the fixed files
@jpetto jpetto requested a review from a team as a code owner June 18, 2024 22:08
Comment on lines -100 to +101
"lint-check": "tsc --noEmit && eslint src/**/*.ts{,x}",
"lint-fix": "tsc --noEmit && eslint src/**/*.ts{,x} --fix",
"lint-check": "eslint src/**/*.ts{,x}",
"lint-fix": "eslint src/**/*.ts{,x} --fix",
Copy link
Contributor

Choose a reason for hiding this comment

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

This would have been set up by us manually back in the day AFAIK, perhaps with the ejection from react-scripts and major version update to ESLint this is no longer needed?

@jpetto jpetto merged commit 066cf41 into main Jun 20, 2024
6 checks passed
@jpetto jpetto deleted the SHIPIT-0618-fix-npm-lint-check-script branch June 20, 2024 14:16
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