Skip to content

Conversation

targos
Copy link
Member

@targos targos commented Oct 25, 2023

No description provided.

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/gyp

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. gyp Issues and PRs related to the GYP tool and .gyp build files needs-ci PRs that need a full CI run. tools Issues and PRs related to the tools directory. labels Oct 25, 2023
@targos
Copy link
Member Author

targos commented Oct 25, 2023

@nodejs/python not sure what to do about https://github.com/nodejs/node/actions/runs/6636731539/job/18029693061

@gengjiawen
Copy link
Member

@nodejs/python not sure what to do about nodejs/node/actions/runs/6636731539/job/18029693061

ignore tools/gyp should do.

"tools/node_modules",

@targos targos added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 25, 2023
@cclauss
Copy link
Contributor

cclauss commented Oct 25, 2023

I would comment out this line in tools/gyp/pyproject.toml
error: TOML parse error at line 94, column 3
|
94 | "PLR1714",
| ^^^^^^^^^
Unknown rule selector: PLR1714

@gengjiawen
Copy link
Member

I would comment out this line: error: TOML parse error at line 94, column 3 | 94 | "PLR1714", | ^^^^^^^^^ Unknown rule selector: PLR1714

In the long run, we shouldn't lint and format deps in Node.js repo. This can be done in gyp-next.

@cclauss
Copy link
Contributor

cclauss commented Oct 25, 2023

Oh... This is because we are using a pinned version of ruff and gyp-next is using a current one.

@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 25, 2023
@cclauss
Copy link
Contributor

cclauss commented Oct 25, 2023

Please update

node/Makefile

Lines 1499 to 1500 in d1ccca9

$(PYTHON) -m pip install --upgrade --target tools/pip/site-packages ruff==0.0.272 || \
$(PYTHON) -m pip install --upgrade --system --target tools/pip/site-packages ruff==0.0.272

@nodejs-github-bot
Copy link
Collaborator

@targos
Copy link
Member Author

targos commented Oct 25, 2023

This should not be related. gyp-next is linted in its own project and I agree with @gengjiawen we should not lint it again with our own rules here.

@cclauss
Copy link
Contributor

cclauss commented Oct 25, 2023

Ruff is designed to work in complex monorepos so we would not be testing the node-gyp directory using this repo’s pyproject.toml (as this error proves). Ruff parsed this file because it will be the settings used to lint that directory.

@anonrig anonrig added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. labels Oct 31, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 31, 2023
@nodejs-github-bot
Copy link
Collaborator

Landed in a77ef54...6557c1c

nodejs-github-bot pushed a commit that referenced this pull request Oct 31, 2023
PR-URL: #50380
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
nodejs-github-bot pushed a commit that referenced this pull request Oct 31, 2023
PR-URL: #50380
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
@targos targos deleted the update-gyp branch October 31, 2023 12:45
alexfernandez pushed a commit to alexfernandez/node that referenced this pull request Nov 1, 2023
PR-URL: nodejs#50380
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
alexfernandez pushed a commit to alexfernandez/node that referenced this pull request Nov 1, 2023
PR-URL: nodejs#50380
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
anonrig pushed a commit to anonrig/node that referenced this pull request Nov 9, 2023
PR-URL: nodejs#50380
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
anonrig pushed a commit to anonrig/node that referenced this pull request Nov 9, 2023
PR-URL: nodejs#50380
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
targos added a commit that referenced this pull request Nov 11, 2023
PR-URL: #50380
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
targos added a commit that referenced this pull request Nov 11, 2023
PR-URL: #50380
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
targos added a commit that referenced this pull request Nov 14, 2023
PR-URL: #50380
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
targos added a commit that referenced this pull request Nov 14, 2023
PR-URL: #50380
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
@targos targos added lts-watch-v18.x lts-watch-v20.x PRs that may need to be released in v20.x labels Nov 15, 2023
UlisesGascon pushed a commit that referenced this pull request Dec 11, 2023
PR-URL: #50380
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
UlisesGascon pushed a commit that referenced this pull request Dec 11, 2023
PR-URL: #50380
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
@UlisesGascon UlisesGascon mentioned this pull request Dec 12, 2023
@richardlau richardlau added backported-to-v20.x PRs backported to the v20.x-staging branch. and removed lts-watch-v20.x PRs that may need to be released in v20.x labels Mar 25, 2024
joshuafried pushed a commit to joshuafried/node that referenced this pull request Sep 13, 2024
PR-URL: nodejs/node#50380
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
aduh95 pushed a commit that referenced this pull request Nov 2, 2024
PR-URL: #50380
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
aduh95 pushed a commit that referenced this pull request Nov 2, 2024
PR-URL: #50380
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backported-to-v20.x PRs backported to the v20.x-staging branch. build Issues and PRs related to build files or the CI. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. gyp Issues and PRs related to the GYP tool and .gyp build files needs-ci PRs that need a full CI run. tools Issues and PRs related to the tools directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants