Skip to content

tools: refactor checkimports.py#50011

Merged
nodejs-github-bot merged 1 commit intonodejs:mainfrom
VoltrexKeyva:refact-py1
Oct 15, 2023
Merged

tools: refactor checkimports.py#50011
nodejs-github-bot merged 1 commit intonodejs:mainfrom
VoltrexKeyva:refact-py1

Conversation

@VoltrexKeyva
Copy link
Member

@VoltrexKeyva VoltrexKeyva commented Oct 2, 2023 •
edited
Loading

  • Use f-strings for formatting.
  • Use raw strings for regexes alongside f-strings.
  • Use a generator.
  • Remove unnecessary else clause.

@nodejs-github-bot nodejs-github-bot added the tools Issues and PRs related to the tools directory. label Oct 2, 2023
@richardlau richardlau added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 2, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 2, 2023
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Oct 2, 2023

CI: https://ci.nodejs.org/job/node-test-pull-request/54459/

@richardlau richardlau added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Oct 2, 2023
@VoltrexKeyva
Copy link
Member Author

VoltrexKeyva commented Oct 4, 2023

@nodejs/python need one more approval to land.

@cclauss
Copy link
Contributor

cclauss commented Oct 4, 2023 •
edited
Loading

I am not a fan of the shebang change. We should join the broad effort to encourage users to use https://docs.python.org/3/library/venv.html to avoid creating dependency conflicts across projects. In a venv, python is defined and points to the same Python version that pip is using to install that project's dependencies.

Today I did brew install python@3.12 and now I can no longer do python3.12 -m pip install --upgrade pip because I get an error: externally-managed-environment which is probably something that many of you Linux users have gotten used to seeing.

The Python core team has worked with OS vendors to lock down the system Python and encourage the use of venv which defines a safe python that is isolated for the dependencies of this project and not all projects on the machine.

@VoltrexKeyva
Copy link
Member Author

VoltrexKeyva commented Oct 7, 2023

@cclauss PTAL.

@VoltrexKeyva VoltrexKeyva added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 9, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 9, 2023
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Oct 9, 2023

CI: https://ci.nodejs.org/job/node-test-pull-request/54631/

@github-actions github-actions bot mentioned this pull request Oct 10, 2023
Open
24 tasks
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Oct 10, 2023

CI: https://ci.nodejs.org/job/node-test-pull-request/54657/

@github-actions github-actions bot mentioned this pull request Oct 11, 2023
Open
30 tasks
@VoltrexKeyva
tools: refactor checkimports.py
a036e93 
- Use f-strings for formatting.
- Use raw strings for regexes alongside f-strings.
- Use a generator.
- Remove unnecessary `else` clause.
@VoltrexKeyva VoltrexKeyva added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 13, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 13, 2023
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Oct 13, 2023

CI: https://ci.nodejs.org/job/node-test-pull-request/54765/

@VoltrexKeyva
Copy link
Member Author

VoltrexKeyva commented Oct 14, 2023

@cclauss will need a re-approval to land.

@cclauss
Copy link
Contributor

cclauss commented Oct 15, 2023 •
edited
Loading

See Merging is blocked below.

I do not know how to translate that into English.

@VoltrexKeyva
Copy link
Member Author

VoltrexKeyva commented Oct 15, 2023

See Merging is blocked below.

I do not know how to translate that into English.

That's intentional, refer to the second point in the collaborator guide's landing pull requests section.

@VoltrexKeyva VoltrexKeyva added the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 15, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 15, 2023
@nodejs-github-bot nodejs-github-bot merged commit 31bde06 into nodejs:main Oct 15, 2023
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Oct 15, 2023

Landed in 31bde06

@VoltrexKeyva VoltrexKeyva deleted the refact-py1 branch October 15, 2023 15:16
kumarrishav pushed a commit to kumarrishav/node that referenced this pull request Oct 16, 2023
@VoltrexKeyva @kumarrishav
tools: refactor checkimports.py
f79cbef 
- Use f-strings for formatting.
- Use raw strings for regexes alongside f-strings.
- Use a generator.
- Remove unnecessary `else` clause.

PR-URL: nodejs#50011
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Christian Clauss <cclauss@me.com>
targos pushed a commit that referenced this pull request Oct 23, 2023
@VoltrexKeyva @targos
tools: refactor checkimports.py
bf7b94f 
- Use f-strings for formatting.
- Use raw strings for regexes alongside f-strings.
- Use a generator.
- Remove unnecessary `else` clause.

PR-URL: #50011
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Christian Clauss <cclauss@me.com>
@targos targos mentioned this pull request Oct 23, 2023
Merged
alexfernandez pushed a commit to alexfernandez/node that referenced this pull request Nov 1, 2023
@VoltrexKeyva @alexfernandez
tools: refactor checkimports.py
7149c88 
- Use f-strings for formatting.
- Use raw strings for regexes alongside f-strings.
- Use a generator.
- Remove unnecessary `else` clause.

PR-URL: nodejs#50011
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Christian Clauss <cclauss@me.com>
targos pushed a commit that referenced this pull request Nov 11, 2023
@VoltrexKeyva @targos
tools: refactor checkimports.py
35b160e 
- Use f-strings for formatting.
- Use raw strings for regexes alongside f-strings.
- Use a generator.
- Remove unnecessary `else` clause.

PR-URL: #50011
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Christian Clauss <cclauss@me.com>
@targos targos mentioned this pull request Nov 12, 2023
Merged
@juanarbol juanarbol mentioned this pull request Dec 6, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cclauss cclauss cclauss approved these changes

@richardlau richardlau richardlau approved these changes

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. tools Issues and PRs related to the tools directory.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@VoltrexKeyva @nodejs-github-bot @cclauss @richardlau