facia-tool
facia-tool copied to clipboard
Run yarn lint and prettier in CI
What's changed?
We in Content Pipeline have had some trouble with different people’s editors disagreeing about formatting, so I’d like to run formatting checks in CI to keep it consistent.
There’s a little more info in the commit messages, reproduced here:
f1e9664413 Run yarn lint in CI
We’ve had some trouble with different people’s editors disagreeing about formatting, so I’d like to make our formatting checks in CI to have it consistent.
5c8fde87f4 Update @typescript-eslint/parser
I was getting a warning that our version of typescript is too new for our version of this package: updating removes the warning.
b3d894b598 Replace tslint with typescript-eslint, updates
TSLint is deprecated, and the suggested replacement is typescript-eslint.
This commit also updates eslint, prettier, and eslint-plugin-prettier, and adds eslint-config-prettier.
64dbdbd912 Run prettier directly rather than via eslint
Prettier’s docs recommend it be run directly rather than via eslint, so this commit removes eslint-plugin-prettier and runs it directly within the lint scripts in package.json.
ed4d99b2a9 Use @guardian/prettier for prettier config
6f84c63b41 Fix .prettierrc symlink in root
I’m guessing the path to fronts-client was updated, and the symlink wasn’t.
2297df1e29 Format everything with prettier
To produce this diff, I ran yarn lint-fix
.
c47958b698 Update .editorconfig files to match prettier
The @guardian/prettier config uses tabs for indentation, so this commit changes the .editorconfig files to match.
(Since the .prettierrc is linked at the root, I think I need to update both .editorconfig files.)
3714938840 Format tables nicely in readme
My editor has some auto-formatting that makes these tables much nicer to read in the markdown source: unless anyone objects I figure it’s an improvement?
9a4f829e33 Update readme
- remove mentions of TSLint
- recommend setting up editor integration for Prettier
793db33d4f Steal isError from tslint
Now that I’ve removed TSLint, this import is failing the build. To fix the build, this commit copies the definition of isError from my local node_modules.
I took a look at https://github.com/guardian/facia-tool/pull/1277, which introduced this, and I didn’t see any explanation for this bit of code: it seems strange to import something from tslint (which is a dev dependency) for this, which makes me wonder if it was deliberate?
Either way, I believe the check is necessary: I initially tried removing it, but that broke some tests.
d9f2322ee5 Remove tslint comments
Now that we’re not using TSLint, these comments aren’t doing anything.
I’ll reintroduce the equivalents for eslint if they’re required.
Implementation notes
- There are some styling changes included in this: for example, the
@guardian/prettier
config that I’ve configured prettier to use uses tabs for indentation rather than spaces.- The config links to https://alexandersandberg.com/articles/default-to-tabs-instead-of-spaces-for-an-accessible-first-environment/ and https://x.com/Rich_Harris/status/1541761871585464323 as explanations of the reason for choosing tabs
- The prettier formatting is all in one commit to make it easier to see what’s changed that isn’t formatting.
Checklist
General
- [ ] 🤖 Relevant tests added
- [ ] ✅ CI checks / tests run locally
- [ ] 🔍 Checked on CODE
Client
- [ ] 🚫 No obvious console errors on the client (i.e. React dev mode errors)
- [ ] 🎛️ No regressions with existing user interactions (i.e. all existing buttons, inputs etc. work)
- [ ] 📷 Screenshots / GIFs of relevant UI changes included