Skip to content

tools: make use of modern syntax in tools/install.py#58167

Closed
avivkeller wants to merge 3 commits intonodejs:mainfrom
avivkeller:patch-510837
Closed

tools: make use of modern syntax in tools/install.py#58167
avivkeller wants to merge 3 commits intonodejs:mainfrom
avivkeller:patch-510837

Conversation

@avivkeller
Copy link
Copy Markdown
Member

@avivkeller avivkeller commented May 4, 2025

From my perspective, this file relies on a few outdated practices:

  • Not using pathlib
  • Not using f-strings

If there is a reason for these outdated practices, feel free to let me know :-)

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. needs-ci PRs that need a full CI run. python PRs and issues that require attention from people who are familiar with Python. labels May 4, 2025
@aduh95 aduh95 added dont-land-on-v20.x dont-land-on-v22.x PRs that should not land on the v22.x-staging branch and should not be released in v22.x. dont-land-on-v24.x PRs that should not land on the v24.x-staging branch and should not be released in v24.x. labels May 4, 2025
Copy link
Copy Markdown
Member

@richardlau richardlau left a comment

Choose a reason for hiding this comment

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

This removes AIX and z/OS code.

@aduh95
Copy link
Copy Markdown
Contributor

aduh95 commented May 6, 2025

If there is a reason for this, feel free to let me know :-)

Shouldn't it be the other way around? If there's a reason not to do this, you should be the one letting us know given you're the one suggesting the change. It's a very large change hard to review, and it's unclear what we get out of it.

@avivkeller
Copy link
Copy Markdown
Member Author

I've simplified. To clarify, my goal was to make this use pathlib, f-strings, a similar.

@avivkeller avivkeller requested a review from richardlau May 8, 2025 19:49
@richardlau richardlau dismissed their stale review May 12, 2025 12:11

Outdated

@avivkeller
Copy link
Copy Markdown
Member Author

Can someone run a. CI so this can land?

@aduh95
Copy link
Copy Markdown
Contributor

aduh95 commented May 21, 2025

/cc @nodejs/python

@aduh95 aduh95 added the stalled Issues and PRs that are stalled. label Nov 8, 2025
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Nov 8, 2025

This issue/PR was marked as stalled, it will be automatically closed in 30 days. If it should remain open, please leave a comment explaining why it should remain open.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Dec 9, 2025

Closing this because it has stalled. Feel free to reopen if this issue/PR is still relevant, or to ping the collaborator who labelled it stalled if you have any questions.

@github-actions github-actions Bot closed this Dec 9, 2025
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. dont-land-on-v22.x PRs that should not land on the v22.x-staging branch and should not be released in v22.x. dont-land-on-v24.x PRs that should not land on the v24.x-staging branch and should not be released in v24.x. needs-ci PRs that need a full CI run. python PRs and issues that require attention from people who are familiar with Python. stalled Issues and PRs that are stalled.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants