Skip to content

Dropper powrap de make verifs #1841

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

Closed
JulienPalard opened this issue Apr 6, 2022 · 14 comments
Closed

Dropper powrap de make verifs #1841

JulienPalard opened this issue Apr 6, 2022 · 14 comments
Labels

Comments

@JulienPalard
Copy link
Member

Il n'est plus utilisé non plus dans la CI de toute façons.

Par contre c'est pas gagné qu'on puisse utiliser awk, la portabilité doit être ... moyenne ... enfin on peut essayer.

@jeanas
Copy link
Collaborator

jeanas commented Apr 28, 2022

Honnêtement, je me demande si à ce stade il ne serait pas plus simple pour tout le monde d'enlever les vérifications de longueur des lignes, et de (EDIT : manquait « de ») make verifs, et de la CI. Et si on tombe sur un diff illisible de temps en temps, on demande juste au contributeur de passer un coup de powrap, ou bien on le fait simplement nous-mêmes. Qu'est ce que vous en pensez ?

@christopheNan
Copy link
Contributor

L'enlever de make verifs ne me dérange pas. En revanche, enlever les vérifications de longueur me perturberait : avec les commits directement dans github, on se retrouve vite avec des (voire beaucoup de) lignes de longueurs variables dont certaines peuvent être très longues.

@jeanas
Copy link
Collaborator

jeanas commented May 1, 2022

Ah oui, c'est vrai. Flûte.

En même temps, on peut imaginer un système où, avant de merger une PR, on lui passe un coup de powrap localement. D'un côté, cela fait une opération pour le « mainteneur », à effectuer pour chaque merge. D'un autre côté, cela évite d'avoir à passer powrap à chaque commit sur sa PR, et enlève une source de complication pour les contributeurs lambda. Dans l'idéal, ce serait fait par PyDocTeur, mais la dernière fois que j'ai fouillé, c'était compliqué…

@jeanas
Copy link
Collaborator

jeanas commented May 1, 2022

Sinon, il y a l'option de remplacer awk par un bête script Python.

@jeanas
Copy link
Collaborator

jeanas commented May 8, 2022

Quelle option préférez-vous @JulienPalard @christopheNan ?

@jeanas
Copy link
Collaborator

jeanas commented May 8, 2022

Au fait, question bête : à quoi sert PyDocTeur maintenant ? :-)

@christopheNan
Copy link
Contributor

Remplacer awk par un script Python, ça s'appelle powrap, non ?
Faire un wrap par un mainteneur, ça modifie l'auteur des commits juste pour une question de forme de fichiers, donc je ne suis pas pour.

@jeanas
Copy link
Collaborator

jeanas commented May 8, 2022

Remplacer awk par un script Python, ça s'appelle powrap, non ?

Non, je parlais juste d'un script qui vérifie la longueur des lignes, sans reformater. powrap --check fait un peu la même chose, sauf qu'il vérifie que lui-même ne changerait pas le fichier, ce qui dépend de la version de gettext installée. Je pensais à faire la même chose que la commande awk qui est dans la CI, mais en Python, à savoir bêtement vérifier que chaque ligne fait moins de 80 caractères ou n'a pas d'espace.

Faire un wrap par un mainteneur, ça modifie l'auteur des commits juste pour une question de forme de fichiers, donc je ne suis pas pour.

Ah non, on rajoute simplement un commit fait par le mainteneur, mais la PR finit squash-mergée, ce qui laisse l'auteur d'origine (GitHub s'arrange pour mettre le mainteneur comme co-auteur dans le message, ce qui est le souvent plus correct car le relecteur aura fait des suggestions, qui auront été appliquées).

@vpoulailleau
Copy link
Contributor

Padpo vérifie en CI la longueur des lignes (sans présupposer d'un formatage éventuel), on a normalement un warning dans la relecture de code.

@jeanas
Copy link
Collaborator

jeanas commented May 8, 2022

Bonne remarque, je n'y avais pas pensé. Par contre, je ne sais pas si les autres mainteneurs font attention à padpo après avoir relu (et pour le coup lu les avertissements padpo) et avant de merger. Pour ma part, je n'en ai pas l'habitude...

@JulienPalard
Copy link
Member Author

D'un côté on a pas essayé awk dans le Makefile. On peut parier sur le fait que si quelq'un a make il a awk ?

@jeanas
Copy link
Collaborator

jeanas commented May 8, 2022

J'en doute. Sous Windows, j'imagine qu'il faut les installer tous les deux. awk fait une dépendance de plus pour les nouveaux.

@vpoulailleau
Copy link
Contributor

Et au pire, ça peut être un bout de Python comme :

import sys
from pathlib import Path

assert not any(
    len(line) > 80
    for line in Path(sys.argv[1]).read_text(encoding="utf-8").splitlines()
)

@jeanas
Copy link
Collaborator

jeanas commented Jun 2, 2022

Ping @JulienPalard @christopheNan ?

JulienPalard added a commit to JulienPalard/python-docs-fr that referenced this issue Oct 15, 2022
JulienPalard added a commit that referenced this issue Oct 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants