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

Make pre-commit usage clearer in error message #2241

Merged
merged 7 commits into from
Apr 20, 2023
Merged

Conversation

msaroufim
Copy link
Member

No description provided.

@msaroufim msaroufim changed the title Make lint error clearer Make pre-commit usage clearer in error message Apr 19, 2023
@msaroufim
Copy link
Member Author

Ok something definitely broken in docker CI https://github.com/pytorch/serve/actions/runs/4739607940/jobs/8414589150?pr=2241 - I'm gonna ignore it to merge my other PRs

@codecov
Copy link

codecov bot commented Apr 19, 2023

Codecov Report

Merging #2241 (257b36c) into master (067fd91) will decrease coverage by 0.12%.
The diff coverage is n/a.

❗ Current head 257b36c differs from pull request most recent head ff9809a. Consider uploading reports for the commit ff9809a to get more accurate results

@@            Coverage Diff             @@
##           master    #2241      +/-   ##
==========================================
- Coverage   71.47%   71.35%   -0.12%     
==========================================
  Files          73       73              
  Lines        3341     3341              
  Branches       57       57              
==========================================
- Hits         2388     2384       -4     
- Misses        950      954       +4     
  Partials        3        3              

see 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@msaroufim msaroufim added the easy label Apr 20, 2023
@@ -76,7 +76,7 @@ jobs:
echo "pip install pre-commit"
echo "cd serve/"
echo "pre-commit install"
echo "pre-commit will lint your code for you, so git add and commit those new changes and this check should become green"
echo "git diff --name-only main | xargs pre-commit run --files"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it was still helpful to let the users know they need to install pre-commit then git add and commit

@msaroufim msaroufim merged commit fa95a66 into master Apr 20, 2023
@msaroufim msaroufim deleted the msaroufim-patch-7 branch April 21, 2023 18:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants