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: replace Python linter flake8 with ruff #47519

Merged
merged 3 commits into from
Apr 14, 2023

Conversation

cclauss
Copy link
Contributor

@cclauss cclauss commented Apr 12, 2023

Ruff supports over 500 lint rules and can be used to replace Flake8 (plus dozens of plugins), isort, pydocstyle, yesqa, eradicate, pyupgrade, and autoflake, all while executing (in Rust) tens or hundreds of times faster than any individual tool.

The ruff Action uses minimal steps to run in ~5 seconds, rapidly providing intuitive GitHub Annotations to contributors.

image

@charliermarsh

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Apr 12, 2023

Review requested:

  • @nodejs/gyp
  • @nodejs/python

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. dont-land-on-v14.x gyp Issues and PRs related to the GYP tool and .gyp build files i18n-api Issues and PRs related to the i18n implementation. icu Issues and PRs related to the ICU dependency. meta Issues and PRs related to the general management of the project. needs-ci PRs that need a full CI run. openssl Issues and PRs related to the OpenSSL dependency. tls Issues and PRs related to the tls subsystem. tools Issues and PRs related to the tools directory. labels Apr 12, 2023
@cclauss cclauss force-pushed the ruff branch 2 times, most recently from 6bba85d to 9da5ea6 Compare April 12, 2023 01:25
@cclauss cclauss changed the title ci: replace Python linter flake8 with ruff build: replace Python linter flake8 with ruff Apr 12, 2023
Copy link
Member

@anonrig anonrig left a comment

Choose a reason for hiding this comment

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

I'm a fan of Ruff. I was planning on doing this change. Happy to see this. Can you share the execution time difference with this change?

@Trott Trott added the python PRs and issues that require attention from people who are familiar with Python. label Apr 12, 2023
@gengjiawen gengjiawen added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 12, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 12, 2023
@nodejs-github-bot
Copy link
Collaborator

@cclauss
Copy link
Contributor Author

cclauss commented Apr 12, 2023

@anonrig Execution time is less than a 1/10th of a second.

node git:(ruff) % time ruff .

ruff .  0.08s user 0.09s system 85% cpu 0.197 total

@gengjiawen
Copy link
Member

gengjiawen commented Apr 12, 2023

node-test-linter failed, not sure extra build steps needed @nodejs/build-infra

@cclauss
Copy link
Contributor Author

cclauss commented Apr 12, 2023

These errors are caused by the mods that are made in the Makefile. Ruff is a Rust exe while flake8 was a Python command line tool. I will convert this PR to DRAFT while I work to fix...

@cclauss cclauss marked this pull request as draft April 12, 2023 09:28
@cclauss cclauss marked this pull request as ready for review April 12, 2023 11:14
Makefile Outdated Show resolved Hide resolved
@nodejs nodejs deleted a comment from tungmeoo Apr 12, 2023
@richardlau richardlau added commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. request-ci Add this label to start a Jenkins CI on a PR. labels Apr 14, 2023
@richardlau
Copy link
Member

The five failures look unrelated but perhaps I am missing something.

We have a number of flaky tests (more since the recent V8 update).

@MoLow MoLow removed tls Issues and PRs related to the tls subsystem. openssl Issues and PRs related to the OpenSSL dependency. i18n-api Issues and PRs related to the i18n implementation. gyp Issues and PRs related to the GYP tool and .gyp build files icu Issues and PRs related to the ICU dependency. labels Apr 14, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 14, 2023
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

Copy link
Member

@gengjiawen gengjiawen left a comment

Choose a reason for hiding this comment

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

Excellent job

@richardlau richardlau added the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 14, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 14, 2023
@nodejs-github-bot nodejs-github-bot merged commit 4c22414 into nodejs:main Apr 14, 2023
@nodejs-github-bot
Copy link
Collaborator

Landed in 4c22414

@cclauss cclauss deleted the ruff branch April 14, 2023 16:52
@cclauss
Copy link
Contributor Author

cclauss commented Apr 14, 2023

Thanks all! -- Its a team sport ;-) If you are involved nodejs/build, please give this a review so I can complete the tour...

targos pushed a commit that referenced this pull request May 2, 2023
PR-URL: #47519
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
@targos targos mentioned this pull request May 2, 2023
danielleadams pushed a commit that referenced this pull request Jul 6, 2023
PR-URL: #47519
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
MoLow pushed a commit to MoLow/node that referenced this pull request Jul 6, 2023
PR-URL: nodejs#47519
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. meta Issues and PRs related to the general management of the project. needs-ci PRs that need a full CI run. python PRs and issues that require attention from people who are familiar with Python. tools Issues and PRs related to the tools directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants