-
Notifications
You must be signed in to change notification settings - Fork 127
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 no-unused-vars ESLint rule #13861
Conversation
The error in the test file was from me messing up the merge, I think
This may be helpful to have - not sure why it isn't used
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ESLint is disabled
vets-website
uses ESLint to help enforce code quality. In most situations we would like ESLint to remain enabled.
What you can do
See if the code can be refactored to avoid disabling ESLint, or wait for a VSP review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Geez, I can't believe how many unused vars made their way in since disabling that rule.
Thanks for doing this painstaking work, @bkjohnson !! |
@department-of-veterans-affairs/va-cto-health-products please review this as soon as you get the chance. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
src/applications/coronavirus-screener/
and src/applications/covid-research-volunteer/
look good
Description
This PR re-enables
no-unused-vars
which was disabled in #12760We are tracking these disabled ESLint rules in department-of-veterans-affairs/va.gov-team#11691
There are a lot of changed files, but I've grouped the apps by commit (in a very loose alphabetical order by app). If you've viewed the changes in the commit for your app and they look good, then you can approve.
The following apps have 2 commits due to further changes after merging in master:
Testing done
Only linting.
Screenshots
Turning the rule on before any of the fixes were made:
Acceptance criteria
Definition of done