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

Add linting using eslint with standard as the base #948

Merged
merged 9 commits into from
May 8, 2024

Conversation

jonathangoulding
Copy link
Collaborator

@jonathangoulding jonathangoulding commented Apr 26, 2024

DEFRA/water-abstraction-team#115

Add linting using eslint with standard as the base

Where possible we want to enforce the coding standards - https://github.com/DEFRA/water-abstraction-team/blob/main/coding_conventions.md

This change introduces eslint using standard as a base (enforcing all the standard js rule). Switching to eslint allows us to extend standard js.

A new command is added to only lint changed files. This will allow us to gradually change and enforce the new linting rules.

@jonathangoulding jonathangoulding force-pushed the lint-add-standards-and-rules-eslint branch 2 times, most recently from 711d2da to fbee471 Compare April 26, 2024 14:35
@Cruikshanks Cruikshanks added the do not merge Used for spikes and experiments label Apr 29, 2024
@jonathangoulding jonathangoulding force-pushed the lint-add-standards-and-rules-eslint branch 2 times, most recently from 6ee8a97 to aaeb3f7 Compare April 30, 2024 08:42
@jonathangoulding jonathangoulding changed the title Lint add standards and rules eslint Add linting using eslint with standard as the base Apr 30, 2024
@jonathangoulding jonathangoulding added the housekeeping Refactoring, tidying up or other work which supports the project label Apr 30, 2024
Where possible we want to enforce the coding standards - https://github.com/DEFRA/water-abstraction-team/blob/main/coding_conventions.md

This change introduces eslint using standard as a base (enforcing all the standard js rule). Switching to eslint allows us to extend standard js.

A new command is added to only lint changed files. This will allow us to gradually change and enforce the new linting rules.
@jonathangoulding jonathangoulding force-pushed the lint-add-standards-and-rules-eslint branch from aaeb3f7 to 802e35a Compare April 30, 2024 08:44
@jonathangoulding jonathangoulding marked this pull request as ready for review April 30, 2024 09:42
@Cruikshanks Cruikshanks removed the do not merge Used for spikes and experiments label Apr 30, 2024
@jonathangoulding jonathangoulding merged commit 32b6230 into main May 8, 2024
6 checks passed
@jonathangoulding jonathangoulding deleted the lint-add-standards-and-rules-eslint branch May 8, 2024 07:16
Cruikshanks added a commit that referenced this pull request May 8, 2024
DEFRA/water-abstraction-team#115

In [Add linting using eslint with standard as the base](#948) we switched from using [standard](https://www.npmjs.com/package/standard) directly for linting to using [ESlint](https://eslint.org/).

This is all part of work we are doing to start codifying our conventions rather than writing them up for no one to read! 😁

As a start, we enabled the rule [max-len](https://eslint.org/docs/latest/rules/max-len) which **standard** doesn't have. That flagged some long text strings we have, and we don't want to break them up just for the sake of the linter.

So, we went to check what options there are for the rule and found this

> This rule was **deprecated** in ESLint v8.53.0. Please use the [corresponding rule](https://eslint.style/rules/js/max-len) in [@stylistic/eslint-plugin-js](https://eslint.style/packages/js).

But it looks like ESLint has moved on a bit since we last used it and has chosen to [move some of its core stylistic based rules to a plugin](https://eslint.style/packages/js).

This change switches to using the plugin to lint `max-len`. It also updates the config to exclude strings and template literals from the rule.
Cruikshanks added a commit that referenced this pull request May 8, 2024
DEFRA/water-abstraction-team#115

In [Add linting using eslint with standard as the base](#948) we switched from using [standard](https://www.npmjs.com/package/standard) directly for linting to using [ESlint](https://eslint.org/).

This is all part of the work we are doing to start codifying our conventions rather than writing them up for no one to read! 😁

As a start, we enabled the rule [max-len](https://eslint.org/docs/latest/rules/max-len) which **standard** doesn't have. That flagged some long text strings we have, and we don't want to break them up just for the sake of the linter.

So, we went to check what options there were for the rule and found this

> This rule was **deprecated** in ESLint v8.53.0. Please use the [corresponding rule](https://eslint.style/rules/js/max-len) in [@stylistic/eslint-plugin-js](https://eslint.style/packages/js).

But it looks like ESLint has moved on a bit since we last used it and has chosen to [move some of its core stylistic-based rules to a plugin](https://eslint.org/blog/2023/10/deprecating-formatting-rules/).

This change switches to using the plugin to lint `max-len`.
Cruikshanks added a commit that referenced this pull request May 8, 2024
DEFRA/water-abstraction-team#115

In [Add linting using eslint with standard as the base](#948) we switched from using [standard](https://www.npmjs.com/package/standard) directly for linting to using [ESlint](https://eslint.org/).

This is all part of the work we are doing to start codifying our conventions rather than writing them up for no one to read! 😁

We then went to make a config change but saw that we were using a deprecated core rule and instead needed to [use @stylistic/eslint-plugin-js for max-len rule](#989). So, we fixed that! 💪

But then we clocked ESLint was telling us our `.eslintrc.js` [config file format is deprecated](https://eslint.org/docs/latest/use/configure/configuration-files-deprecated)! 😩 We instead should be using the `eslint.config.js` [flat file config](https://eslint.org/docs/latest/use/configure/configuration-files).

So, before we start making changes to our config this change gets us using the latest supported format.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
housekeeping Refactoring, tidying up or other work which supports the project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants