Skip to content

Ajout de l'usage 'powrap' pour l'indentation des fichiers #1764

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
wants to merge 1 commit into from

Conversation

Fipaddict
Copy link
Contributor

Suite à la discussion sur https://discuss.afpy.org/t/resolu-erreur-1-lors-du-wrap/587 , je propose d'ajouter l'usage de la commande powrap -m pour réindenter les fichier (vs make wrap qui ne fait que vérifier)

@jeanas
Copy link
Collaborator

jeanas commented Nov 3, 2021

Puisque nous en sommes à discuter de cela, pourquoi est-ce que make wrap ne ferait pas tout simplement powrap -m ? En tous cas, moi, j'ai trouvé le fait que cela ne fasse que vérifier totalement contre-intuitif. C'est sûrement utile pour la CI, mais la commande est déjà séparée.

@JulienPalard
Copy link
Member

+1 pour changer make wrap et lui faire rewrapper, ça veut dire qu'il faut changer make verifs pour lui faire juste executer powrap avec le --check quand même, on est 3 a trouver contre-intuitif le fait que make wrap utilise le --check.

@Fipaddict
Copy link
Contributor Author

Votre proposition me semble aussi plus intéressante que ma PR (qui sera alors complètement hors sujet).
Du coup, je la ferme ? Quelle est la bonne pratique sur le sujet ?

@jeanas
Copy link
Collaborator

jeanas commented Nov 4, 2021

C'est comme vous voulez : soit vous vous sentez de changer cette PR pour faire ce qui a été résolu, et vous rajoutez simplement un autre commit dans la branche, soit vous préférez le laisser à quelqu'un d'autre (je peux m'en occuper sans problème) et dans ce cas vous pouvez fermer la PR.

@Fipaddict
Copy link
Contributor Author

Je ne maitrise pas le Makefile... Je ferme donc la PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants