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

build: Allow pre-commit to keep changes in reformatted code #3604

Merged
merged 1 commit into from
Apr 21, 2024

Conversation

terrytangyuan
Copy link
Member

Removing the two args --check, --diff that prevent pre-commit from writing the changes back:

  --check                         Don't write the files back, just return the
                                  status. Return code 0 means nothing would
                                  change. Return code 1 means some files would
                                  be reformatted. Return code 123 means there
                                  was an internal error.
  --diff                          Don't write the files back, just output a
                                  diff to indicate what changes Black would've
                                  made. They are printed to stdout so

Writing the changes back would be more convenient since developers don't have to run black commands locally. They only need to do git commit ... once pre-commit is installed.

Signed-off-by: Yuan Tang <terrytangyuan@gmail.com>
@oss-prow-bot oss-prow-bot bot requested review from ckadner and yuzisun April 15, 2024 15:18
@spolti
Copy link
Contributor

spolti commented Apr 15, 2024

WHen CI fails because lint, will it show what is different?
Or it this case, it is not importante as now it will write things?

@terrytangyuan
Copy link
Member Author

An example:

pre-commit run
Helm Docs............................................(no files to check)Skipped
black....................................................................Failed
- hook id: black
- files were modified by this hook

reformatted python/huggingfaceserver/huggingfaceserver/async_generate_stream.py

All done! ✨ 🍰 ✨
1 file reformatted.

Flake8...................................................................Passed

No diff will be shown but I don't think it's needed either since they will be fixed automatically.

You can see the diff via git diff.

@spolti
Copy link
Contributor

spolti commented Apr 18, 2024

/lgtm

@yuzisun
Copy link
Member

yuzisun commented Apr 21, 2024

/approve

Copy link

oss-prow-bot bot commented Apr 21, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: spolti, terrytangyuan, yuzisun

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@yuzisun yuzisun merged commit af37834 into kserve:master Apr 21, 2024
55 of 56 checks passed
@terrytangyuan terrytangyuan deleted the fix-commit branch April 21, 2024 14:30
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.

None yet

3 participants