-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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: update ruff and add lint-py-fix
#54410
Conversation
tools/v8-json-to-junit.py
Outdated
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.
(Review note) Ref: astral-sh/ruff#12455
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.
lgtm
CC @nodejs/python |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #54410 +/- ##
==========================================
- Coverage 88.06% 87.77% -0.29%
==========================================
Files 651 651
Lines 183538 183538
Branches 35862 35527 -335
==========================================
- Hits 161635 161109 -526
- Misses 15142 15678 +536
+ Partials 6761 6751 -10 |
No objections in a week, how is everyone feeling about this? |
Its never ending... |
Once this lands? Given that ruff keeps updating, another one shouldn't be opened until the next semver-minor, WDYT @cclauss P.S. can this land? |
(This modifies the Makefile, I don't know why I ever removed I've also requested a CI, hopefully that's okay. |
I know ruff 0.6.3 is out, but I don't think it's worth it to upgrade ruff again until the next semver-minor. |
@nodejs/python I have given this pull request a positive review and I think it should be merged but I am uncertain him to remove the Merging is blocked comment below. |
IIUC that notice is to prevent accidental merging from the GitHub UI (not the CQ) |
What is a |
Apologies, the Commit Queue |
This comment was marked as outdated.
This comment was marked as outdated.
CI LGTM - The failure is related to an incident with github that has since been resolved. |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
CI LGTM |
CI is passing, can this land? |
Ruff is currently at 0.6.7, however, this has a passing CI, and ruff get's updated all the time. For that reason, I'm not updating ruff again. IMO it can be done once v0.7 is released. |
This comment has been minimized.
This comment has been minimized.
Commit Queue failed- Loading data for nodejs/node/pull/54410 ✔ Done loading data for nodejs/node/pull/54410 ----------------------------------- PR info ------------------------------------ Title build: update ruff and add `lint-py-fix` (#54410) Author Aviv Keller <[email protected]> (@RedYetiDev) Branch RedYetiDev:ruff-0.6.0 -> nodejs:main Labels build, python, author ready, needs-ci, commit-queue-squash Commits 1 - build: update ruff and add `lint-py-fix` Committers 1 - RedYetiDev <[email protected]> PR-URL: https://github.com/nodejs/node/pull/54410 Reviewed-By: Christian Clauss <[email protected]> ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/54410 Reviewed-By: Christian Clauss <[email protected]> -------------------------------------------------------------------------------- ⚠ Commits were pushed since the last approving review: ⚠ - build: update ruff and add `lint-py-fix` ℹ This PR was created on Fri, 16 Aug 2024 16:57:16 GMT ✔ Approvals: 1 ✔ - Christian Clauss (@cclauss): https://github.com/nodejs/node/pull/54410#pullrequestreview-2258871347 ✔ Last GitHub CI successful ℹ Last Full PR CI on 2024-09-21T16:23:14Z: https://ci.nodejs.org/job/node-test-pull-request/62637/ - Querying data for job/node-test-pull-request/62637/ ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ Aborted `git node land` session in /home/runner/work/node/node/.ncuhttps://github.com/nodejs/node/actions/runs/11001786432 |
(Sorry for the ping, rerequesting review so this can land) |
Landed in a1cd3c8 |
PR-URL: #54410 Reviewed-By: Christian Clauss <[email protected]>
PR-URL: nodejs#54410 Reviewed-By: Christian Clauss <[email protected]>
PR-URL: nodejs#54410 Reviewed-By: Christian Clauss <[email protected]>
This PR updates
ruff
to 0.6.5, and adds two new makefile targets:lint-py-fix
lint-py-fix-unsafe