10 July 2023

Local code quality checks should be opt-in

About a month into a new job I’m realising that most of my day-to-day workspace management techniques, and almost all of my testing and debugging tactics, trigger errors in our TypeScript build, eslint UI and/or Git hooks.

For example:

  • I wanted to test an error state in the app, and the easiest way seemed to be to add || true to an if condition. TypeScript wouldn’t let me, because the fact that this made the first guard clause always fire made the second guard clause definitely never fire – and this meant that the happy path, even though now doubly unreachable, was using a variable that was possibly undefined.

    I tried commenting all of that code out. This time eslint complained that there were unused variables, and our config treated that as an error, preventing the code from going through. I added lines simply referring to the mentioned variables, which allowed the build to proceed. I uncommented the happy path and put everything back – and got another error. WebStorm had removed some imports that had become unnecessary with the happy path commented, so I had to reinstate them before continuing.

  • I recently ran into another eslint error and tried to bypass it by removing everything from .eslintrc.js. This got rid of all of our rules, which caused all existing // eslint-ignore comments to trigger errors about rules not being found.

  • debugger; is not allowed.

  • If eslint or some other code quality check is configured as a local git hook, it will throw an error whenever I try to use the git history as a temporary store for patches, such as for copying a subset of the current workdir changes to another branch.

I know there are “better ways to do it” in each of those cases. That’s not the point – the point is that each of those things is a sub-step within a procedure to achieve a particular end-state, and those are just the tools I’ve got into the habit of using to get there. Having a code quality check interrupt me in the middle of that process is highly frustrating – it’s like not being allowed to type functio in your editor because that’s not a valid keyword.

The ideal scenario, to my mind, is this:

  • A remote hook and/or PR check enforces code quality standards on important branches like main or development.

  • Developers can optionally introduce checks that happen sooner, e.g. as part of the live-reloading dev build and/or in local commit hooks.

  • By default, the local dev code and any unprotected branches – both local and remote – are not subject to such checks and are seen as tools for the developer to use as they see fit in order to write code and organise their Git history.

Checks that are there by default can be disabled locally by modifying the configuration, but that’s not a desirable solution because it leaves the devs who want them off in a state of having bypassed code quality checks – with the implication that if something gets through, it’s their fault for playing fast and loose with code quality. The checks that are needed should be located as close as possible to the branches that need them – ie. the remote versions of main etc – and there shouldn’t be a practice of anyone bypassing them except in unusual situations.