Consider not running pre-commit in GitHub actions but instead splitting each pre-commit test in a GitHub action step

[Mortinat]

Je pense qu'utiliser les pre-commit dans les test ne soit pas quelque chose de tres judicieux surtout que certains hooks modifie les fichier. Il serait plus intéressant de setup les test qu'on veux directement dans le github actions (ex: flake8/balck, etc)

[C4ptainCrunch]

Quand tu dis "utiliser les pre-commit dans les test" tu parles de faire tourner pre-commit dans les github actions c'est ça ?

[Mortinat]

Oui

[C4ptainCrunch]

Faire run le même pre-commit en local et en CI est clairement un pattern pour lequel le tool a été designé, au point qu'il existe une GitHub action et un tool écrits par l'auteur pour faire ça

surtout que certains hooks modifie les fichier.

Si je ne me trompe pas, en tournant dans github actions, il ne fait pas les modifications, juste print un diff puis stop la CI (en tout cas, je suis certain qu'il ne fait pas de commit avec les modifs)

Il serait plus intéressant de setup les test qu'on veux directement dans le github actions (ex: flake8/balck, etc)

Ca veut dire avoir 2 sources de vérité pour ce qu'on veut comme linter. Et potentiellement avoir un commit qui passe en local puis qui fail sur GitHub. Ca double le travail à chaque update de version ou à chaque fois qu'on ajoute ou supprime un pre-commit.

[Mortinat]

Personnellement je trouve que ca rend les tests moins lisible. Mais si on decide de quand meme utilise pre-commit dans les tests il faudrait utiliser l'action dédier a ca. Et techniquement c'est pas 2 source de vérité car ce sont le meme tools avec les meme versions qui vont run. Je trouve que la difference est minime dans un cas moins de travail mais moins lisible dans l'autre un peu plus de travail mais plus lisible

[C4ptainCrunch]

Mais si on decide de quand meme utilise pre-commit dans les tests il faudrait utiliser l'action dédier a ca.

Bien vu, je pensais que c'était le cas mais non. Je vais fix ça

Et techniquement c'est pas 2 source de vérité car ce sont le meme tools avec les meme versions qui vont run.

Comment tu fais pour run les mêmes tests avec les mêmes version d'un côté avec le tool pre-commit et de l'autre avec une action par tool ? Genre j'ajoute mypy à mon pre-commit.yaml, comment il se met tout seul dans le github action ? Si on sait résoudre ce problème alors je suis pas contre utiliser cette solution là :)

[Mortinat]

mypy run déjà a part. mais prenons l'exemple de black il suffirait d'utiliser - uses: psf/black@stable dans l'action et quand on met a jour pre-commit on met a jour l'action. D'ailleurs pour certains action meme pas besoin car ca se met tous seul a jour genre black. Et c'est un process qu'on devra dans tout les cas faire car comme @titouanc le mentionnais ne pas avoir les meme version en requirement et en pre-commit sa pose soucis

[C4ptainCrunch]

quand on met a jour pre-commit on met a jour l'action.

C'est ce que j'appelle avoir 2 sources de vérité et que j'essaie de limiter

car ca se met tous seul a jour genre black

Je préfère éviter la magie. Demain si black (ou mypy ou autre) fait une upgrade et se met à linter des trucs qu'il ne détectait pas avant je n'ai pas envie de la test suite casse. Sinon, le premier dev qui commit après l'upgrade se prend la CI qui fail alors qu'il n'y peut rien. Ou pire, une PR passe les tests, puis l'upgrade sliencieuse arrive puis on merge dans master et c'est master qui fail soudaiement alors que la PR était OK (j'ai déjà eu le coup, c'est pénible). Btw, c'est pour ça que pre-commit refuse (ou fait un gros warning, je sais plus) d'avoir la version == main dans la config.

ne pas avoir les meme version en requirement et en pre-commit sa pose soucis

Les tools qu'on a dans pre-commit ne sont pas dans les requirements, normalement y'a pas de doublon (sinon, indeed, c'est avoir 2 sources de vérité et c'est pénible)

mypy run déjà a part

Parce que mypy est trop lent pour tourner dans un pre-commit et surtout, il a besoin de tous les packages installés pour tourner correctement. Ce n'est pas parce qu'il y en a un qui est différent que tout le système est à jeter imo