Skip to content

Commit

Permalink
Run prettier and pre-commit hooks only on staged files (#613)
Browse files Browse the repository at this point in the history
## 🔧 Problem

Currently, the pre-commit hook managed by husky run prettier on all the
files for each commit, even if the files were not changed. It slows down
the dev process unnecessarily most of the time.

## 🍰 Solution

Run linting only on staged files when commiting.

## 🚨  Points to watch / comments

My first attempt was to keep using husky with
https://github.com/lint-staged/lint-staged. It works well for prettier
but doesn't work for ruff as it doesn't manage `pipenv run` nor
`virtualenv` correctly. It looks very "javascript centric".

My second and final attempt was to use https://pre-commit.com/. It's
written in Python, it can manage every language (with hooks or using
shell commands) and is well suited for our environment as we already use
`pipenv` and Python.

`npm` commands removed:
- `format:check` => replaced by `lint:all` and changed in the CI file

`npm` commands added:
- `lint:prettier` lint one file using prettier. Used by other npm
commands.
- `lint:prettier:all` lint all files in the project using prettier.
- `lint:ruff:all` lint all python files using ruff check and format
(with sort option selected).
- `lint:all` prettier and ruff lint on all files.
- `fix:prettier` lint and fix the errors using prettier for one file
(used in pre-commit hook)
- `fix:prettier:all` lint and fix the errors using prettier for all the
files.
- `fix:ruff:all` lint all python files using ruff check and format (with
sort option selected) and fix the errors
- `fix:all` prettier and ruff fix on all files


A `pyproject.toml` file was added to tell ruff to not check the `data/`
directory.

I've added some simple checks with pre-commit like yaml format,
line-ending checks (avoid mixing windows/linux/macox line ending),
trailing-spaces removal and a check to prevent commiting directly on
master (I suppose the git repo is already configured to avoid that, but
I like to have it locally before I push).

## :desert_island: How to test

First, uninstall husky:

    npx husky uninstall

Check that the command `git config --get core.hookspath` returns
nothing. If it gives you something like `.husky/\_` unset it manually
using `git config --unset-all core.hooksPath`

Install pre-commit:

    pipenv install
    pipenv run pre-commit install

Test that it's working as expected by issuing the following command:

    pipenv run pre-commit run --all-files

You should get something like that:

![image](https://github.com/MTES-MCT/ecobalyse/assets/154904/80f20211-9791-425f-a32c-e293a04ea8d6)
  • Loading branch information
vjousse authored Jun 5, 2024
1 parent e5d81ed commit 36c446e
Show file tree
Hide file tree
Showing 13 changed files with 301 additions and 133 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/node.js.yml
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,8 @@ jobs:
- name: Build Elm static Db
run: npm run db:build

- name: Run prettier formatting check
run: npm run format:check
- name: Run prettier & ruff formatting check
run: npm run lint:all

- name: Run elm-review
run: npm run test:review
Expand Down
31 changes: 31 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v2.3.0
hooks:
- id: check-yaml
- id: end-of-file-fixer
exclude: (svg|icomoon|rapidoc-9.3.4.min.js)
- id: trailing-whitespace
exclude: (vendor|rapidoc-9.3.4.min.js)
- id: no-commit-to-branch
name: "don't commit to master"
args: [--branch, master]

- repo: https://github.com/charliermarsh/ruff-pre-commit
rev: v0.4.7
hooks:
# Run the linter.
- id: ruff
args: [ --select, I, --fix, --exit-non-zero-on-fix ]
# Run the formatter.
- id: ruff-format

- repo: local
hooks:
- id: app-prettier
name: run prettier
language: system
files: ^.*$
types_or: [javascript, json]
entry: |
bash -c 'npm --prefix . run fix:prettier --write "${@}"' --
2 changes: 2 additions & 0 deletions .prettierignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,5 @@ public/icomoon/*
public/vendor/*
styles.scss
tests/e2e-*-output.json
dist/
Pipfile.lock
1 change: 1 addition & 0 deletions Pipfile
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,6 @@ gunicorn = ">=21,<22"
psycopg = ">=3.1,<3.2"
python-decouple = ">=3,<4"
ruff = "*"
pre-commit = "*"

[dev-packages]
346 changes: 236 additions & 110 deletions Pipfile.lock

Large diffs are not rendered by default.

27 changes: 13 additions & 14 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,31 +55,30 @@ Trois instances de développement sont alors accessibles :
- [localhost:8001](http://localhost:8001/) sert l'API ;
- [localhost:1234](http://localhost:1234/) est l'URL à utiliser en développement pour tester l'intégration des trois composants (le front, l'API et le Django) car un proxy Parcel renvoie certaines requêtes vers le port 8001 ou 8002 (voir `.proxyrc`). Le frontend est servi en mode _hot-reload_, pour recharger! l'interface Web à chaque modification du code frontend.

### Hooks Git avec Husky et Formatage de Code avec Prettier
### Hooks Git avec pre-commit et Formatage de Code avec Prettier et Ruff

Ce projet utilise Husky pour gérer les hooks Git, et Prettier pour le formatage automatique du code.
Le build sur le CI échouera si les fichiers javascript et json ne sont pas proprement formattés.
Ce projet utilise https://pre-commit.com/ pour gérer les hooks Git ainsi que Prettier et Ruff pour le formatage automatique du code.
Le build sur le CI échouera si les fichiers python, javascript et json ne sont pas proprement formattés.

#### Pré-requis
#### Vérification Automatique avant chaque Commit

- Husky
- Prettier
Pour installer les hooks pre-commit, exécutez la commande suivante :

Si vous clonez le dépôt pour la première fois, les dépendances devraient être installées automatiquement après avoir exécuté npm install. Si ce n'est pas le cas, vous pouvez les installer manuellement.
$ pipenv run pre-commit install

$ npm install --save-dev husky prettier
Un hook de pre-commit sera alors configuré pour vérifier que le code est bien formaté avant de permettre le commit. Le hook corrigera les erreurs dans la mesure du possible. Il vous suffira alors d'ajouter les modifications à votre staging, git puis à refaire votre commit.

#### Vérification Automatique avant chaque Commit
Il est possible de lancer la vérification du formatage à la main grâce à la commande suivante :

Un hook de pre-commit a été configuré pour vérifier que le code est bien formaté avant de permettre le commit. Si le code n'est pas correctement formaté, le commit sera bloqué.
$ npm run lint:all

Pour résoudre ce problème, vous pouvez exécuter la commande suivante :
Si vous voulez lancer la correction automatique de tous les problèmes détectés, lancez :

$ npm run format:json
$ npm run fix:all

Si vous ne souhaitez pas que la vérification se fasse de manière automatique, vous pouvez désinstaler les hooks :
Si vous ne souhaitez pas que la vérification se fasse de manière automatique, vous pouvez désinstaller pre-commit et les hooks associés :

$ npx husky uninstall
$ pipenv run pre-commit uninstall

## Compilation

Expand Down
1 change: 0 additions & 1 deletion data/.gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,2 @@
/*.csv
/*.zip

2 changes: 1 addition & 1 deletion data/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ format:
npm run format:json

export_food_format: export_food format


#export_textile:
# @if [ "$(shell docker container inspect -f '{{.State.Running}}' $(NAME) )" = "true" ]; then \
Expand Down
2 changes: 0 additions & 2 deletions data/bwapi/README.txt
Original file line number Diff line number Diff line change
Expand Up @@ -62,5 +62,3 @@ https://bwapi.ecobalyse.fr/food/methods/EF%20v3.1/acidification/accumulated%20ex
* The characterization factors of a method

https://bwapi.ecobalyse.fr/food/characterization_factors/EF%20v3.1/acidification/accumulated%20exceedance%20(AE)


2 changes: 1 addition & 1 deletion data/docker/entrypoint.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
ECOBALYSE_ID=$(ls -lnd /home/jovyan/ecobalyse|awk '{print $3}')
JOVYAN_ID=$(id -u jovyan)

if [ $ECOBALYSE_ID -ne $JOVYAN_ID ]; then
if [ $ECOBALYSE_ID -ne $JOVYAN_ID ]; then
usermod -u $ECOBALYSE_ID jovyan
fi

Expand Down
9 changes: 8 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,15 @@
"build:init": "./bin/update-version.sh && mkdir -p dist && cp -r public/* dist/ && npm run db:build",
"processes:build": "elm make src/ComputeAggregated.elm --output=compute-aggregated-app.js && node compute-aggregated.js",
"db:build": "./bin/build-db",
"lint:prettier": "prettier --config .prettierrc --check",
"lint:prettier:all": "npm run lint:prettier -- .",
"lint:ruff:all": "pipenv run ruff check --extend-select I && pipenv run ruff format --check",
"lint:all": "npm run lint:prettier:all && npm run lint:ruff:all",
"fix:prettier": "npm run lint:prettier -- --write",
"fix:prettier:all": "npm run lint:prettier -- --write .",
"fix:ruff:all": "pipenv run ruff check --extend-select I --fix && pipenv run ruff format",
"fix:all": "npm run fix:ruff:all && npm run fix:prettier:all",
"format:json": "npx [email protected] --write . && pipenv run ruff check --select I --fix && pipenv run ruff format",
"format:check": "npx [email protected] --check . && pipenv run ruff check --select I && pipenv run ruff format --check",
"prepare": "husky",
"server:build": "npm run db:build && elm make src/Server.elm --optimize --output=server-app.js",
"server:dev": "npm run server:build && nodemon server.js --config nodemon.json",
Expand Down
5 changes: 5 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[project]
requires-python = ">= 3.11"

[tool.ruff]
exclude = ["data/"]
2 changes: 1 addition & 1 deletion src/Static/Json.elm-template
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ transportsJson =


processes : Processes
processes =
processes =
{ foodProcesses = foodProcessesJson
, textileProcesses = textileProcessesJson
}
Expand Down

0 comments on commit 36c446e

Please sign in to comment.