Skip to content
This repository was archived by the owner on Aug 7, 2025. It is now read-only.

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 subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants