Skip to content

Commit

Permalink
Use prettier to manage code style and formatting (#1476)
Browse files Browse the repository at this point in the history
DEFRA/water-abstraction-team#115

Now, we are a team of seven, with seven different opinions on the 'right' way to write the code! Because of our size, we are splitting into two teams to focus on various features simultaneously. But we'll still be working with the one code base.

We'd already moved from just using [StandardJS](https://standardjs.com/) to lint our code to using **StandardJS** via **ESLint** (we kept the rules but not the tool) because there are too many cases where **StandardJS** has no ruling, but we wanted one.

However, each time these rules don't provide a style convention, the team must stop, discuss, debate, and finally decide how something will be done. We want something else to take the decision away from us!

Step forward [Prettier](https://prettier.io/). **Prettier** is an opinionated code formatter that focuses only on the style of the code.

> Prettier enforces a consistent code style (i.e. code formatting that won’t affect the AST) across your entire codebase because it disregards the original styling by parsing it away and re-printing the parsed AST with its own rules that take the maximum line length into account, wrapping code when necessary.

So, any rules or conventions we have that would affect the [Abstract Syntax Tree (AST)](https://www.nearform.com/insights/what-is-an-abstract-syntax-tree/) would still be controlled by **ESlint**. These are the things as a team it is worth spending time debating and agreeing.

For the rest, we intend to let **Prettier** take over. It is widely used across the JavScript community and is popularly advocated for teams that become large or dispersed like ours.

Our approach to the adoption was first to remove our existing **ESlint** config, add **Prettier**, and then let it update all the files.

Then, having checked through as a team there were no 'red-line' changes, commit them. We then add **ESlint** back in and only re-apply the non-stylistic rules.

Our research found that JSDoc is still better managed through **ESlint** (**Prettier** does nothing with it), so we also added our original config for that.

Another fundamental change is that we are no longer bringing in **StandardJS** as an **ESlint** extension. This means we can move to the latest **ESlint** v9 and away from the [deprecated config file format](https://eslint.org/docs/latest/use/configure/configuration-files-deprecated) to the new [flat file config](https://eslint.org/docs/latest/use/configure/configuration-files).

Previously, we'd been blocked because extensions are not supported in ESLint v9. **StandardJS** does provide [eslint-config-standard](https://github.com/standard/eslint-config-standard), but along with the core project, it does not appear to be actively maintained, is still using the deprecated **ESLint** style rules and has an error so cannot be used.

> See [Switch to new eslint config format](#991) for more details on why this was blocking us

We'd resigned ourselves to manually copying and updating the rules from their plugin when we revisited an old issue when first switching to **ESLint** with **StandardJS**. [Support for Eslint v9 Flat Config format](standard/eslint-config-standard#411) was one of the blockers that meant to have both **ESLint** and **StandardJS** play ball, we were stuck on ESLint v8.

We spotted that there had been some [recent activity](standard/eslint-config-standard#411 (comment)), all of which referenced an alternative called [neostandard](https://github.com/neostandard/neostandard).

And oh gosh! All our dreams were answered.

- Built for ESlint to avoid the need for separate IDE tooling
- Built for the latest ESLint (v9), so flat-file config is supported
- Just like we did, any style rules have been updated to use @stylistic/eslint-plugin
- An explicit desire to work with current practices. So, built for use with ESLint only, banning or requiring `;` is now an option, and disabling style rules for those opting to use **prettier** is possible

For context, maintenance on **StandardJS** and related packages like **eslint-config-standard** has been stalled for some time. **neostandard** references the issue as being a [block in governance and direction of travel](standard/standard#1948 (comment)). I've not been through every message, but it appears the maintainers are split between those who remained committed to StandardJS's 'one-tool one-way' approach and those looking to move to where most folks are: **ESLint**.

Even our own @johnwatson484 [has gotten involved!](standard/standard#1948 (comment)).

We're betting this isn't going to be resolved any time soon, so to avoid us having to maintain standard rules in our ESLint config manually, the final fundamental change to highlight is we're now using ESLint + neostandard to manage coding rules.
  • Loading branch information
Jozzey authored Nov 22, 2024
1 parent 4fc8eaf commit 1be5dea
Show file tree
Hide file tree
Showing 1,283 changed files with 14,720 additions and 14,588 deletions.
97 changes: 0 additions & 97 deletions .eslintrc.js

This file was deleted.

22 changes: 11 additions & 11 deletions .github/dependabot.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@

version: 2
updates:
- package-ecosystem: npm
directory: "/"
schedule:
interval: daily
open-pull-requests-limit: 10
versioning-strategy: lockfile-only
- package-ecosystem: "github-actions"
# Workflow files stored in the default location of `.github/workflows`
directory: "/"
schedule:
interval: "daily"
- package-ecosystem: npm
directory: '/'
schedule:
interval: daily
open-pull-requests-limit: 10
versioning-strategy: lockfile-only
- package-ecosystem: 'github-actions'
# Workflow files stored in the default location of `.github/workflows`
directory: '/'
schedule:
interval: 'daily'
4 changes: 2 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ jobs:
- name: Checkout repository
uses: actions/checkout@v4
with:
fetch-depth: 0 # Shallow clones should be disabled for a better relevancy of sonarcloud analysis
fetch-depth: 0 # Shallow clones should be disabled for a better relevancy of sonarcloud analysis

# Before we do anything, check we haven't accidentally left any `describe.only()` or `it.only(` statements in the
# tests
Expand All @@ -85,7 +85,7 @@ jobs:
- name: Install Node
uses: actions/setup-node@v4
with:
node-version-file: ".nvmrc"
node-version-file: '.nvmrc'

# Speeds up workflows by reading the node modules from cache. Obviously you need to run it at least once, and the
# cache will be updated should the package-lock.json file change
Expand Down
1 change: 0 additions & 1 deletion .jsdoc.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

'use strict'

module.exports = {
Expand Down
39 changes: 33 additions & 6 deletions .labrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,40 @@ module.exports = {
// of globals to ignore to be a single comma-delimited string; for ease of management we define them in an array then
// join them.
globals: [
'__extends', '__assign', '__rest', '__decorate', '__param', '__metadata', '__awaiter', '__generator',
'__exportStar', '__createBinding', '__values', '__read', '__spread', '__spreadArrays', '__spreadArray', '__await',
'__asyncGenerator', '__asyncDelegator', '__asyncValues', '__makeTemplateObject', '__importStar', '__importDefault',
'__classPrivateFieldGet', '__classPrivateFieldSet', '__esDecorate', '__runInitializers', '__propKey',
'__setFunctionName', '__classPrivateFieldIn', '__addDisposableResource', '__disposeResources',
'__extends',
'__assign',
'__rest',
'__decorate',
'__param',
'__metadata',
'__awaiter',
'__generator',
'__exportStar',
'__createBinding',
'__values',
'__read',
'__spread',
'__spreadArrays',
'__spreadArray',
'__await',
'__asyncGenerator',
'__asyncDelegator',
'__asyncValues',
'__makeTemplateObject',
'__importStar',
'__importDefault',
'__classPrivateFieldGet',
'__classPrivateFieldSet',
'__esDecorate',
'__runInitializers',
'__propKey',
'__setFunctionName',
'__classPrivateFieldIn',
'__addDisposableResource',
'__disposeResources',
// We also ignore globals exposed by global-agent:
'GLOBAL_AGENT', 'ROARR',
'GLOBAL_AGENT',
'ROARR',
// GlobalNotifier is added by us a global in a server plugin. It's how we make logging available anywhere in the app
// whilst avoiding having to pass it around
'GlobalNotifier',
Expand Down
6 changes: 6 additions & 0 deletions .prettierrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"semi": false,
"singleQuote": true,
"trailingComma": "none",
"printWidth": 120
}
14 changes: 7 additions & 7 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ Generated by [`auto-changelog`](https://github.com/CookPete/auto-changelog).
- Exclude VOID returns from two-part tariff billing [`#1419`](https://github.com/DEFRA/water-abstraction-system/pull/1419)
- 2PT use full authorised volume rather than Nil [`#1404`](https://github.com/DEFRA/water-abstraction-system/pull/1404)
- Bump @aws-sdk/client-s3 from 3.670.0 to 3.673.0 [`#1417`](https://github.com/DEFRA/water-abstraction-system/pull/1417)
- Rename gauging stations to monitoring stations [`#1415`](https://github.com/DEFRA/water-abstraction-system/pull/1415)
- Rename gauging stations to monitoring stations [`#1415`](https://github.com/DEFRA/water-abstraction-system/pull/1415)
- Fix auth scope on view monitoring station route [`#1413`](https://github.com/DEFRA/water-abstraction-system/pull/1413)
- Import licence document for a licence [`#1392`](https://github.com/DEFRA/water-abstraction-system/pull/1392)
- Bump eslint-plugin-jsdoc from 50.3.2 to 50.4.1 [`#1411`](https://github.com/DEFRA/water-abstraction-system/pull/1411)
Expand Down Expand Up @@ -339,10 +339,10 @@ Generated by [`auto-changelog`](https://github.com/CookPete/auto-changelog).
- Add 'currentVersion' modifier to LicenceModel [`#1133`](https://github.com/DEFRA/water-abstraction-system/pull/1133)
- Fix returns requirement button text [`#1134`](https://github.com/DEFRA/water-abstraction-system/pull/1134)
- Feature-requirements-for-returns-view-page [`#1118`](https://github.com/DEFRA/water-abstraction-system/pull/1118)
- Add orderBy to data fetched for two-part tariff review [`#1131`](https://github.com/DEFRA/water-abstraction-system/pull/1131)
- Add orderBy to data fetched for two-part tariff review [`#1131`](https://github.com/DEFRA/water-abstraction-system/pull/1131)
- Add new two-part tariff generate bill run endpoint [`#1123`](https://github.com/DEFRA/water-abstraction-system/pull/1123)
- Add created_by to return versions and link models [`#1126`](https://github.com/DEFRA/water-abstraction-system/pull/1126)
- Update two-part tariff review pages to add Cypress data test attributes [`#1128`](https://github.com/DEFRA/water-abstraction-system/pull/1128)
- Update two-part tariff review pages to add Cypress data test attributes [`#1128`](https://github.com/DEFRA/water-abstraction-system/pull/1128)
- Fix Ret. Req. logic to determine cycle for 2PT [`#1124`](https://github.com/DEFRA/water-abstraction-system/pull/1124)
- Bump joi from 17.13.1 to 17.13.3 [`#1127`](https://github.com/DEFRA/water-abstraction-system/pull/1127)
- Update validation msg for ret. req. copy existing [`#1125`](https://github.com/DEFRA/water-abstraction-system/pull/1125)
Expand Down Expand Up @@ -433,7 +433,7 @@ Generated by [`auto-changelog`](https://github.com/CookPete/auto-changelog).
- Select additional submission options page [`#988`](https://github.com/DEFRA/water-abstraction-system/pull/988)
- Refactor purposes page to use purpose ids as values [`#1025`](https://github.com/DEFRA/water-abstraction-system/pull/1025)
- Bump sass from 1.77.1 to 1.77.2 [`#1030`](https://github.com/DEFRA/water-abstraction-system/pull/1030)
- Add sentence case function & rename capitalize [`#1029`](https://github.com/DEFRA/water-abstraction-system/pull/1029)
- Add sentence case function & rename capitalize [`#1029`](https://github.com/DEFRA/water-abstraction-system/pull/1029)
- Fix QA issues found with the review adjustment factors validation [`#1027`](https://github.com/DEFRA/water-abstraction-system/pull/1027)
- Refactor to use computed error message [`#1026`](https://github.com/DEFRA/water-abstraction-system/pull/1026)
- Remove returns requirements button on check page [`#1021`](https://github.com/DEFRA/water-abstraction-system/pull/1021)
Expand Down Expand Up @@ -672,7 +672,7 @@ Generated by [`auto-changelog`](https://github.com/CookPete/auto-changelog).
- Add legacy create bill run request [`#800`](https://github.com/DEFRA/water-abstraction-system/pull/800)
- Return instead of await requests [`#799`](https://github.com/DEFRA/water-abstraction-system/pull/799)
- Move legacy refresh requests to module [`#798`](https://github.com/DEFRA/water-abstraction-system/pull/798)
- Migrate Charging Module services to *.request.js [`#797`](https://github.com/DEFRA/water-abstraction-system/pull/797)
- Migrate Charging Module services to \*.request.js [`#797`](https://github.com/DEFRA/water-abstraction-system/pull/797)
- Add defra-user-id header to legacy requests [`#796`](https://github.com/DEFRA/water-abstraction-system/pull/796)
- Move non-model helpers out of helpers [`#795`](https://github.com/DEFRA/water-abstraction-system/pull/795)
- Move currentFinancialYear() from test [`#794`](https://github.com/DEFRA/water-abstraction-system/pull/794)
Expand Down Expand Up @@ -723,7 +723,7 @@ Generated by [`auto-changelog`](https://github.com/CookPete/auto-changelog).
- Move SendTransactions to root with refactoring [`#745`](https://github.com/DEFRA/water-abstraction-system/pull/745)
- Remove Landing Page From System [`#747`](https://github.com/DEFRA/water-abstraction-system/pull/747)
- Fix & tidy test DB legacy migrations [`#725`](https://github.com/DEFRA/water-abstraction-system/pull/725)
- Returns required - Select an existing return requirement from basic page [`#746`](https://github.com/DEFRA/water-abstraction-system/pull/746)
- Returns required - Select an existing return requirement from basic page [`#746`](https://github.com/DEFRA/water-abstraction-system/pull/746)
- Bump dotenv from 16.4.4 to 16.4.5 [`#744`](https://github.com/DEFRA/water-abstraction-system/pull/744)
- Determine match and allocate issues and status [`875f478`](https://github.com/DEFRA/water-abstraction-system/commit/875f4780b35ba4007d0d7a9c370c8329e0832488)

Expand Down Expand Up @@ -886,7 +886,7 @@ Generated by [`auto-changelog`](https://github.com/CookPete/auto-changelog).
- Bump @aws-sdk/client-s3 from 3.472.0 to 3.473.0 [`#588`](https://github.com/DEFRA/water-abstraction-system/pull/588)
- Add new page for add a note [`#587`](https://github.com/DEFRA/water-abstraction-system/pull/587)
- Requirement approval page [`#585`](https://github.com/DEFRA/water-abstraction-system/pull/585)
- Add check your answers page for return required journey [`#584`](https://github.com/DEFRA/water-abstraction-system/pull/584)
- Add check your answers page for return required journey [`#584`](https://github.com/DEFRA/water-abstraction-system/pull/584)
- Create `fetch-licences` service [`#577`](https://github.com/DEFRA/water-abstraction-system/pull/577)
- Add check your answers page (return requirements) [`#579`](https://github.com/DEFRA/water-abstraction-system/pull/579)
- Bump @aws-sdk/client-s3 from 3.470.0 to 3.472.0 [`#583`](https://github.com/DEFRA/water-abstraction-system/pull/583)
Expand Down
25 changes: 8 additions & 17 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,28 +12,17 @@ This API provides an interface for calculating charges, queuing transactions and

Make sure you already have:

- [Node.js v16.*](https://nodejs.org/en/)
- [PostgreSQL v12](https://www.postgresql.org/)
- [Node.js v20.\*](https://nodejs.org/en/)
- [PostgreSQL v14](https://www.postgresql.org/)

## Installation
## Running locally

First clone the repository and then drop into your new local repo:

```bash
git clone https://github.com/DEFRA/water-abstraction-system.git && cd water-abstraction-system
```

Create the databases:

```bash
npm run create-db
npm run create-test-db
```

Our preference is to run the database and API within Docker, so [install Docker](https://docs.docker.com/get-docker/) if you don't already have it.
This is one of a number of apps that make up the Water Resource Licencing service. Because of the service's complex infrastructure there is a separate project available that will build a fully working WRLS environment, using [Docker](https://docs.docker.com/get-docker/). We recommend reaching out to the [Water Abstraction team](https://github.com/orgs/DEFRA/teams/water-abstraction) and requesting access to **wal-dev-environment** if you need to get this project up and running.

## Configuration

> This is automatically setup when running locally using **wal-dev-environment**
Any configuration is expected to be driven by environment variables when the service is run in production as per [12 factor app](https://12factor.net/config).

However when running locally in development mode or in test it makes use of the [Dotenv](https://github.com/motdotla/dotenv) package. This is a shim that will load values stored in a `.env` file into the environment which the service will then pick up as though they were there all along.
Expand All @@ -48,6 +37,8 @@ If you have an idea you'd like to contribute please log an issue.

All contributions should be submitted via a pull request.

The code style is dictated by [Prettier](https://prettier.io/), and we follow [StandardJS](https://standardjs.com/) code rules as implemented by [neostandard](https://github.com/neostandard/neostandard). This is all managed through [ESLint](https://eslint.org/) so should play nice with all IDE's.

## License

THIS INFORMATION IS LICENSED UNDER THE CONDITIONS OF THE OPEN GOVERNMENT LICENCE found at:
Expand Down
6 changes: 3 additions & 3 deletions app/controllers/bill-licences.controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const RemoveBillLicenceService = require('../services/bill-licences/remove-bill-
const SubmitRemoveBillLicenceService = require('../services/bill-licences/submit-remove-bill-licence.service.js')
const ViewBillLicenceService = require('../services/bill-licences/view-bill-licence.service.js')

async function remove (request, h) {
async function remove(request, h) {
const { id } = request.params

const pageData = await RemoveBillLicenceService.go(id)
Expand All @@ -22,7 +22,7 @@ async function remove (request, h) {
})
}

async function submitRemove (request, h) {
async function submitRemove(request, h) {
const { id } = request.params

try {
Expand All @@ -34,7 +34,7 @@ async function submitRemove (request, h) {
}
}

async function view (request, h) {
async function view(request, h) {
const { id } = request.params

const pageData = await ViewBillLicenceService.go(id)
Expand Down
Loading

0 comments on commit 1be5dea

Please sign in to comment.