-
Notifications
You must be signed in to change notification settings - Fork 17
[Build] Add linters #6
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
Changes from all commits
cbf1a03
111f8e2
ffe8983
0aed554
2d4e925
605a2da
65464bf
d6412a9
bcb4126
349b356
24d5723
9aeaceb
faa15fe
fbf76da
abe7ed1
f1098fb
c42ccf3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| name: Linters, using linux | ||
| on: | ||
| workflow_dispatch: | ||
| pull_request: | ||
| branches: | ||
| - main | ||
| jobs: | ||
| build: | ||
| name: Linters | ||
| runs-on: ubuntu-22.04 | ||
| steps: | ||
| - name: Python check | ||
| uses: actions/setup-python@v4 | ||
| with: | ||
| python-version: '3.10' | ||
| - uses: actions/checkout@v4 | ||
| - name: Run linters | ||
| run: | | ||
| bash .github/workflows/scripts_new/linters.sh | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| #!/bin/bash | ||
|
|
||
| set -ex | ||
|
|
||
| python -V | ||
| pwd | ||
| ls | ||
| uname -a | ||
|
|
||
| # python + C++ | ||
| # ============= | ||
|
|
||
| pip install pre-commit | ||
| pre-commit run -a --show-diff | ||
|
|
||
| # python | ||
| # ====== | ||
|
|
||
| pip install pyright | ||
| # Need to deal with C++ linkage issues first (and possibly | ||
| # some other things), before we can turn on pyright | ||
|
|
||
| pip install isort | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do you think of using ruff? We already use it in
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like ruff
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should merge all CI PRs first , before running a
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (possibly excluding this one I suppose 🤔 )
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well... I suppose most of the changes are to either C++ or CI, which are both orthogonal to ruff 🤔 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we could start using ruff now and only enable isort rules instead of migrating later, but up to you. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One advantage I see with ruff is that it has well-maintained pre-commit hooks and Github actions, so I think it'd allow removing some code in this PR, especially since it would also allow removing black and pylint in addition to isort.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh interesting. didnt realize There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, but I'd favor configuring in
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yup, good point. thanks! 🙌 |
||
| # TODO: run isort on all python files, and commit those, then | ||
| # uncomment the following line: | ||
| # isort --check-only --diff python | ||
|
|
||
| # C++ | ||
| # === | ||
|
|
||
| # TODO: figure out how to run clang-tidy | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same again.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ruff for instance has an official well maintained github action. I think it'd make sense to use it, so that what we have to maintain is minimal.