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: Add flake8 and black to pre-commit hooks #3578

Merged
merged 4 commits into from
Apr 9, 2024

Conversation

terrytangyuan
Copy link
Member

@terrytangyuan terrytangyuan commented Apr 5, 2024

For local development, these checks would run automatically when committing changes and we can easily add additional checks/hooks.

@oss-prow-bot oss-prow-bot bot requested review from ckadner and lizzzcai April 5, 2024 15:21
Signed-off-by: Yuan Tang <terrytangyuan@gmail.com>
@terrytangyuan
Copy link
Member Author

Need to update based on 6e88700

Copy link
Contributor

@spolti spolti left a comment

Choose a reason for hiding this comment

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

Signed-off-by: Yuan Tang <terrytangyuan@gmail.com>
Signed-off-by: Yuan Tang <terrytangyuan@gmail.com>
Signed-off-by: Yuan Tang <terrytangyuan@gmail.com>
@@ -28,9 +28,8 @@ jobs:
- name: Lint with flake8
uses: py-actions/flake8@v2
with:
args: "--config python/.flake8"
args: "--config .flake8"
Copy link
Member Author

Choose a reason for hiding this comment

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

Moved .flake8 file to top level to work with pre-commit

@sivanantha321
Copy link
Member

I believe it is a good thing to add the pre-commit as a dev dependency. WDYT ?

@terrytangyuan
Copy link
Member Author

I believe it is a good thing to add the pre-commit as a dev dependency. WDYT ?

Good idea but I think it should only be recommended but not required. I can submit a PR to the website later.

.flake8 Show resolved Hide resolved
@sivanantha321
Copy link
Member

/lgtm

@lizzzcai
Copy link
Member

lizzzcai commented Apr 8, 2024

/lgtm

@yuzisun
Copy link
Member

yuzisun commented Apr 9, 2024

/approve

Copy link

oss-prow-bot bot commented Apr 9, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lizzzcai, sivanantha321, 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

@oss-prow-bot oss-prow-bot bot added the approved label Apr 9, 2024
@yuzisun yuzisun merged commit 19314ab into kserve:master Apr 9, 2024
109 checks passed
tjandy98 pushed a commit to tjandy98/kserve that referenced this pull request Apr 10, 2024
* build: Add flake8 and black to pre-commit hooks

Signed-off-by: Yuan Tang <terrytangyuan@gmail.com>

* fix path

Signed-off-by: Yuan Tang <terrytangyuan@gmail.com>

* pass config

Signed-off-by: Yuan Tang <terrytangyuan@gmail.com>

* fix flake8

Signed-off-by: Yuan Tang <terrytangyuan@gmail.com>

---------

Signed-off-by: Yuan Tang <terrytangyuan@gmail.com>
Signed-off-by: tjandy98 <3953059+tjandy98@users.noreply.github.com>
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

5 participants