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

[TECH] Ne pas permettre de créer de nouveau usecase dans api/lib (PIX-10667) #7865

Closed
wants to merge 4 commits into from

Conversation

yannbertrand
Copy link
Member

🎄 Problème

Dans le cadre de la migration vers les Bounded Contexts, on a pour le moment pas d'outillage pour nous empêcher de créer des nouveaux usecases dans api/lib/domain/usecases/.

🎁 Proposition

Pour éviter ça, la proposition est d'ajouter une nouvelle configuration ESLint spécifiquement pour les fichiers ajoutés dans l'historique et qui se trouvent dans le mauvais dossier. Pour assurer une erreur, on active une règle de lint : le nombre de lignes maximum de fichiers doit être inférieur à 0.

Script lint:staged

Pour appliquer le lint sur les fichiers qui nous intéressent uniquement, on utilise :

git log --name-only --format='' --diff-filter=A dev..HEAD | grep 'api/lib/domain/usecases/' | sed -e 's|api/||g'
  1. on récupère les noms des fichiers ajoutés (et rien d'autres) entre la branche locale dev et l'état courant,
  2. on filtre les noms qui contiennent api/lib/domain/usecases/,
  3. on supprime api/ du nom du fichier car on est déjà dans ce dossier lors du lint.

Cela donne une suite de noms de fichier du genre : lib/domain/usecases/fake-usecase.js.

Cette liste est envoyée à ESLint via :

eslint --config .eslint-new-files-in-lib-config.cjs --no-eslintrc ${...}

Ou on applique notre configuration particulière.

Fichier de config ESLint

On spécifie les mêmes paramètres de parsing que notre config de base (pour bien interpréter le JS), et on applique la fameuse règle : 'max-lines': ['error', 0],

🧦 Remarques

C'est une proposition qui a ses limites :

  • La commande ne renvoie pas d'erreur tant que le fichier n'a pas été commité,
  • Elle ne traite que les créations de fichiers, pas les renommages ou déplacements,

🎅 Pour tester

Un mauvais fichier a été ajouté en fin d'historique git, la CI doit être cassée sur celui ci à cause de notre nouvelle commande lint:staged.

@yannbertrand yannbertrand added the cross-team Toutes les équipes de dev label Jan 16, 2024
@yannbertrand yannbertrand self-assigned this Jan 16, 2024
@pix-bot-github
Copy link

Une fois les applications déployées, elles seront accessibles via les liens suivants :

Les variables d'environnement seront accessibles via les liens suivants :

@yannbertrand yannbertrand force-pushed the tech-lint-staged branch 5 times, most recently from 1399db1 to 0c5962c Compare January 16, 2024 16:34
@yaf
Copy link
Contributor

yaf commented Feb 29, 2024

Cette PR a l'air très bien et semble fonctionner correctement. Il manque encore un truc pour la passer en « ready for review » ? Voir pour la déployer ?

@yannbertrand
Copy link
Member Author

Cette PR a l'air très bien et semble fonctionner correctement. Il manque encore un truc pour la passer en « ready for review » ? Voir pour la déployer ?

Je pense que la description n'est pas tout à fait synchro avec ce que le code fait vraiment. Mais j'ai totalement oublié où j'en étais donc faudrait reprendre le sujet tranquillement...

@yannbertrand
Copy link
Member Author

Clôt par manque d'activité

@yannbertrand yannbertrand deleted the tech-lint-staged branch March 18, 2024 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bounded-context-migration cross-team Toutes les équipes de dev
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants