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

Add precommit hook #198

Closed
wants to merge 7 commits into from
Closed

Add precommit hook #198

wants to merge 7 commits into from

Conversation

RonanMorgan
Copy link
Collaborator

@RonanMorgan RonanMorgan commented May 21, 2024

Install precommit hook, the same configuration as in pyro-engine.

@RonanMorgan RonanMorgan self-assigned this May 21, 2024
Copy link

codecov bot commented May 21, 2024

Codecov Report

Attention: Patch coverage is 66.66667% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 87.69%. Comparing base (59d2029) to head (ab3103d).
Report is 1 commits behind head on develop.

Files Patch % Lines
pyroengine/core.py 0.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #198      +/-   ##
===========================================
- Coverage    87.92%   87.69%   -0.24%     
===========================================
  Files            6        6              
  Lines          381      382       +1     
===========================================
  Hits           335      335              
- Misses          46       47       +1     
Flag Coverage Δ
unittests 87.69% <66.66%> (-0.24%) ⬇️

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.

Copy link
Member

@MateoLostanlen MateoLostanlen left a comment

Choose a reason for hiding this comment

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

Hi @RonanMorgan, thnaks for your PR ! please can you add the setup-docker-compose.sh script to the script folder and generate a docker-compose.override.yml ignore by git that will be read automatically in order to avoid changing a file track by git

@MateoLostanlen MateoLostanlen added the type: enhancement New feature or request label May 21, 2024
Copy link
Member

@MateoLostanlen MateoLostanlen left a comment

Choose a reason for hiding this comment

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

Hello, thanks for the update.

Please do not change stuff that have nothing to do with this PR you should not format engine.py here

@RonanMorgan
Copy link
Collaborator Author

RonanMorgan commented May 28, 2024

@MateoLostanlen sorry at the beginning I thought that it would be very small (to add precommit hook) and there is already so much PR open so that I prefered move forward

@MateoLostanlen
Copy link
Member

@RonanMorgan what do you mean you're not going to continue with this PR?

@RonanMorgan RonanMorgan changed the title Limits the ressources available for the container Add precommit hook May 29, 2024
@RonanMorgan
Copy link
Collaborator Author

@MateoLostanlen :
ok so the "limits" part is now here : #203

I am gonna keep this PR for the static check part

@RonanMorgan
Copy link
Collaborator Author

TODO : if we use the precommit hook, we should remove the linting part in the makefile and also clean up the workflows...

@RonanMorgan RonanMorgan marked this pull request as draft May 29, 2024 09:30
@RonanMorgan RonanMorgan deleted the rs/add-container-limits branch June 11, 2024 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants