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.

Sign up for GitHub

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

Jump to bottom

Improve quality check and contributing guide #3430

Merged
merged 22 commits into from
Jan 27, 2023
Merged

Improve quality check and contributing guide #3430

merged 22 commits into from
Jan 27, 2023

Conversation

submarcos
Copy link
Member

@submarcos submarcos commented Jan 25, 2023 •
edited
Loading

  • Set new docs/CONTRIBUTING.rst file included in doc.
  • Split dev dependencies, add pip-tools and flake8 to them
  • Create a pre-commit git hook to check problems
  • Improve CI to use same versions as developpers to check Flake8 and requirements (dependabot will update them)
  • Reorder and fix documentation compilation errors (+ fixes and check in CI)

next:

  • create issue / ask feature templates

@camillemonchicourt
Copy link
Member

Merci !

@camillemonchicourt
Copy link
Member

Peut-être mentionner plus explicitement que tout nouveau développement doit être couvert par des tests, et qu'une PR ne doit pas faire baisser la couverture de test pour être acceptée ?

@submarcos
Copy link
Member Author

submarcos commented Jan 25, 2023 •
edited
Loading

j'ai mis tout çà dans le fichier de doc, mais peut etre faut il etre plus insistant ou plus clair ?

@cypress
Copy link

cypress bot commented Jan 25, 2023 •
edited
Loading

Passing run #5362 ↗︎

0 24 0 0 Flakiness 0

Details:

Merge 708d824 into e6523e4...
Project: Geotrek-admin Commit: 1f24a0f292 ℹ️
Status: Passed Duration: 03:26 💡
Started: Jan 27, 2023 10:14 AM Ended: Jan 27, 2023 10:18 AM

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

@codecov
Copy link

codecov bot commented Jan 25, 2023 •
edited
Loading

Codecov Report

Base: 98.26% // Head: 98.26% // No change to project coverage 👍

Coverage data is based on head (708d824) compared to base (e6523e4).
Patch has no changes to coverable lines.

Additional details and impacted files 
@@           Coverage Diff           @@
##           master    #3430   +/-   ##
=======================================
  Coverage   98.26%   98.26%           
=======================================
  Files         291      291           
  Lines       20869    20869           
=======================================
  Hits        20506    20506           
  Misses        363      363

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@submarcos submarcos requested a review from a team January 25, 2023 15:22
@submarcos submarcos mentioned this pull request Jan 25, 2023
Merged
marcantoinedupre
marcantoinedupre reviewed Jan 25, 2023
View reviewed changes
.githooks/pre-commit

echo "run Flake8..."

dockercompose run -T --rm web flake8 geotrek
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about checking changed files only?

Copy link
Member Author

@submarcos submarcos Jan 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't check changed files, we check all files as actually

marcantoinedupre
marcantoinedupre approved these changes Jan 25, 2023
View reviewed changes
Copy link
Contributor

@marcantoinedupre marcantoinedupre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice 👍

Comme suggeré par Célia, un pointeur vers la section Contribution de la doc dans le readme serait pertinent je pense.

@camillemonchicourt
Copy link
Member

La norme est surtout d'avoir un fichier CONTRIBUTING.md à la racine du dépôt : https://docs.github.com/fr/communities/setting-up-your-project-for-healthy-contributions/setting-guidelines-for-repository-contributors#adding-a-contributing-file

Quitte à ce que celui-ci ne contienne qu'un lien vers la rubrique de contribution dans la doc officielle.
Un peu comme https://github.com/django/django/blob/main/CONTRIBUTING.rst

OK, pour les tests, j'avais pas vu cette ligne "Unit-tests total coverage is above or at least equal with previous commits. Patch coverage is 100% on new lines.", donc en effet c'est assez clair.

@submarcos
Copy link
Member Author

ok je vais voir pour mettre çà dans docs/CONTRIBUTING.rst pour qu'il soit à la fois dans la doc et qui remonte sur le dépot comme il faut

@submarcos submarcos force-pushed the improve_quality branch 3 times, most recently from 17be266 to 6b30c63 Compare January 26, 2023 14:34
@submarcos
Copy link
Member Author

Nice +1

Comme suggeré par Célia, un pointeur vers la section Contribution de la doc dans le readme serait pertinent je pense.

je vais le faire, mais je vais d'abord attendre que la doc soit publiée pour avoir le lien (apres le merge / release)

@submarcos submarcos marked this pull request as ready for review January 26, 2023 15:10
@submarcos submarcos requested a review from a team January 26, 2023 15:10
@submarcos submarcos merged commit 6240a97 into master Jan 27, 2023
@LePetitTim LePetitTim deleted the improve_quality branch January 27, 2023 10:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LePetitTim LePetitTim LePetitTim approved these changes

@marcantoinedupre marcantoinedupre marcantoinedupre approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@submarcos @camillemonchicourt @marcantoinedupre @LePetitTim