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

fix: update black parameter to meet line-length of 100 chars requirement #2847

Merged
merged 2 commits into from
Jan 24, 2024

Conversation

edelclaux
Copy link
Contributor

@edelclaux edelclaux commented Jan 3, 2024

[Périmètre] lint python 99 vs 100

[Problème]
La doc stipule un line-length de 100, mais la valeur par défaut de black est fixée dans le projet à 99

Doc: https://docs.geonature.fr/development.html#backend
image

Project configuration pyproject.toml:

[tool.black]
line-length = 99

[Solution]
Forcer le lint à la valeur stipulée dans la doc

Remarque --> si 100 n'est plus une valeur satisfaisante, on peut faire varier la valeur doc et le pyproject conjointement

@edelclaux edelclaux changed the title fix: update black parameter to meet line-mength of 100 chars requirement fix: update black parameter to meet line-length of 100 chars requirement Jan 3, 2024
Copy link

codecov bot commented Jan 3, 2024

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (14ad7c6) 78.43% compared to head (cc8b301) 78.43%.

Files Patch % Lines
backend/geonature/app.py 0.00% 1 Missing ⚠️
backend/geonature/core/gn_commons/repositories.py 0.00% 1 Missing ⚠️
backend/geonature/core/gn_meta/routes.py 0.00% 1 Missing ⚠️
...kend/geonature/core/users/register_post_actions.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #2847   +/-   ##
========================================
  Coverage    78.43%   78.43%           
========================================
  Files           88       88           
  Lines         7183     7183           
========================================
  Hits          5634     5634           
  Misses        1549     1549           
Flag Coverage Δ
pytest 78.43% <63.63%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jacquesfize
Copy link
Contributor

jacquesfize commented Jan 9, 2024

Tu peux me donner l'autorisation de push sur ta branche ? J'ai fix le lint dans plusieurs fichiers du backend.

@edelclaux
Copy link
Contributor Author

edelclaux commented Jan 9, 2024

Tu peux me donner l'autorisation de push sur ta branche ? J'ai fix le lint dans plusieurs fichiers du backend.

Comme c'est un changement de la config du lint du backend, j'ai volontairement pas linté les fichiers.
Comme ça modifier une petite 30aine de fichier, il vaudrait mieux le faire juste avant de merger non ?

(Je viens de pousser la version avec un commit de lintage, qui se drope si besoin)

@jacquesfize
Copy link
Contributor

Ah si tu as fait les modifs, c'est tout bon. Par contre, tu as fait une modification sur le module d'authentification ?

@edelclaux
Copy link
Contributor Author

non, effectivement. Je sais pas d'où est sortie cette modif. C'est corrigé

@edelclaux
Copy link
Contributor Author

Du coup, attention, t'as généré une branche fix/lint sur le repo pnx.
image

Ça veux dire que vous travaillez directement sur le repo ?

Je pensais que vous aviez un fork 'ecrins' et que vous faisiez des PR

@jacquesfize
Copy link
Contributor

jacquesfize commented Jan 9, 2024

Une erreur de ma part :s J'ai crée la branche par erreur et je dois la supprimer. [ C'est fait ! ]

@jacquesfize jacquesfize added this to the 2.14 milestone Jan 24, 2024
@jacquesfize jacquesfize merged commit 014281e into PnX-SI:develop Jan 24, 2024
6 of 7 checks passed
@edelclaux edelclaux deleted the fix/lint branch January 24, 2024 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants