What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Changes how linting works:
Prettier / eslint no longer run as part of npm test
The TypeScript compiler is no longer acting as another linter (though it is still on strict mode)
Prettier / eslint now run their formatting functions as pre-commit hooks
Prettier / eslint also run as part of CI on PRs just in case
I've tested the workflow changes work by pushing a commit that has linting errors and as expected the checks fail during the linting step.
The choices made for this PR are totally up for discussion. I took steps that made the most sense to me for setting up linting, but we might want to do it a completely different way!
Other information:
Initially we were thinking of removing either Prettier or eslint since having both seemed redundant. However researching it further, I now understand why we'd want have both installed on the project. I think that the current setup for these linting and formatting tools is still a bit clunky though; ideally for running unit tests locally I do not want to run these tools while my code is still in a WIP state. They really should only be run when finishing up the commit and during CI, so I've modified the setup to now run the prettier and eslint autofixing function as a pre-commit hook (meaning in theory we shouldn't even really have to deal with most lint errors, they'll just be fixed automatically :crossed_fingers:). I've also now added a step in CI to run the lint checks since they've been removed from npm test script.
I've also modified our tsconfig.json to disable some more strict errors like unused variables and such that are handled by eslint in a way that doesn't break our tests when code is in a WIP state.
npm testI've tested the workflow changes work by pushing a commit that has linting errors and as expected the checks fail during the linting step.
The choices made for this PR are totally up for discussion. I took steps that made the most sense to me for setting up linting, but we might want to do it a completely different way!
npm testscript.I've also modified our
tsconfig.jsonto disable some more strict errors like unused variables and such that are handled byeslintin a way that doesn't break our tests when code is in a WIP state.Resolves PER-8621. Also resolves #33.