build: use pathlib for paths by VoltrexKeyva · Pull Request #47581 · nodejs/node

VoltrexKeyva · 2023-04-16T14:21:56Z

Use Python's pathlib library for paths and related operations instead of os.path.

Refs: #47323 (comment)#47323 (comment)

targos · 2023-04-17T13:38:30Z

@nodejs/python

nodejs-github-bot · 2023-04-18T19:52:39Z

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

nodejs-github-bot · 2023-04-21T07:48:48Z

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

nodejs-github-bot · 2023-04-21T15:22:08Z

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

nodejs-github-bot · 2023-04-21T18:12:08Z

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

nodejs-github-bot · 2023-04-21T21:25:01Z

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

cclauss · 2023-04-22T05:07:48Z

@VoltrexKeyva Can you please create a pull request for all the non-pathlib changes so we can see if we can get that one to pass the tests and land first?

nodejs-github-bot · 2023-05-02T14:23:08Z

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

VoltrexKeyva · 2023-05-02T16:33:58Z

@cclauss fortunately there are no failures, should we land this now?

cclauss · 2023-05-02T16:36:38Z

Nice! Two approvals are above.

nodejs-github-bot · 2023-05-02T16:54:49Z

Commit Queue failed

- Loading data for nodejs/node/pull/47581
✔ Done loading data for nodejs/node/pull/47581
----------------------------------- PR info ------------------------------------
Title build: use pathlib for paths (#47581)
Author Mohammed Keyvanzadeh (@VoltrexKeyva)
Branch VoltrexKeyva:use-pathlib -> nodejs:main
Labels build, python, author ready, needs-ci
Commits 1
- build: use pathlib for paths
Committers 1
- Mohammed Keyvanzadeh PR-URL: https://github.com/nodejs/node/pull/47581
Reviewed-By: Yagiz Nizipli Reviewed-By: Christian Clauss ------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/47581
Reviewed-By: Yagiz Nizipli Reviewed-By: Christian Clauss --------------------------------------------------------------------------------
⚠ Commits were pushed since the last review:
⚠ - build: use pathlib for paths
ℹ This PR was created on Sun, 16 Apr 2023 14:21:56 GMT
✔ Approvals: 2
✔ - Yagiz Nizipli (@anonrig): https://github.com/nodejs/node/pull/47581#pullrequestreview-1388073872
✔ - Christian Clauss (@cclauss): https://github.com/nodejs/node/pull/47581#pullrequestreview-1388196158
✔ Last GitHub CI successful
ℹ Last Full PR CI on 2023-05-02T14:23:08Z: https://ci.nodejs.org/job/node-test-pull-request/51578/
- Querying data for job/node-test-pull-request/51578/
✔ Last Jenkins CI successful
--------------------------------------------------------------------------------
✔ Aborted `git node land` session in /home/runner/work/node/node/.ncu

https://github.com/nodejs/node/actions/runs/4863726456

VoltrexKeyva · 2023-05-02T18:48:39Z

@cclauss this'll need an approval again, as new changes invalidates the older approvals.

nodejs-github-bot · 2023-05-02T19:20:24Z

Landed in d2156f1

cclauss · 2023-05-02T19:54:06Z

Rock & Roll... Thanks for doing this!

Use Python's `pathlib` library for paths and related operations instead of `os.path`. Refs: #47323 (comment)#47323 (comment) PR-URL: #47581 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Christian Clauss <cclauss@me.com>

Use Python's `pathlib` library for paths and related operations instead of `os.path`. Refs: nodejs#47323 (comment)nodejs#47323 (comment) PR-URL: nodejs#47581 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Christian Clauss <cclauss@me.com>

srl295 · 2024-10-01T14:35:23Z

 # --with-icu-source processing
 # now, check that they didn't pass --with-icu-source=deps/icu
- elif with_icu_source and os.path.abspath(icu_full_path) == os.path.abspath(with_icu_source):
+ elif with_icu_source and Path(icu_full_path).resolve() == Path(with_icu_source).resolve():


Ouch! icu_full_path can actually be a URL here, and so this change breaks Windows downloading completely. Not good..

nodejs-github-bot added build Issues and PRs related to build files or the CI. needs-ci PRs that need a full CI run. labels Apr 16, 2023

VoltrexKeyva added the python PRs and issues that require attention from people who are familiar with Python. label Apr 16, 2023

anonrig reviewed Apr 17, 2023
View reviewed changes

Comment threadconfigure.py Outdated

VoltrexKeyva force-pushed the use-pathlib branch from 6d9d20c to 495b1fcCompare April 17, 2023 08:04

anonrig approved these changes Apr 17, 2023
View reviewed changes

cclauss approved these changes Apr 17, 2023
View reviewed changes

Comment threadconfigure.py Outdated

VoltrexKeyva force-pushed the use-pathlib branch from 495b1fc to aa2fe83Compare April 17, 2023 16:35

Trott added request-ci Add this label to start a Jenkins CI on a PR. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels Apr 18, 2023

github-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 18, 2023

VoltrexKeyva force-pushed the use-pathlib branch from aa2fe83 to fbc3909Compare April 18, 2023 21:26

VoltrexKeyva force-pushed the use-pathlib branch from fbc3909 to 289a07dCompare April 21, 2023 07:55

VoltrexKeyva added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 21, 2023

github-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 21, 2023

VoltrexKeyva force-pushed the use-pathlib branch from 289a07d to 66e508aCompare April 21, 2023 15:47

VoltrexKeyva added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 21, 2023

github-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 21, 2023

This was referenced Apr 22, 2023
build: refactor configure.py #47667
Merged
build: refactor configure.py #47323
Closed

This was referenced Apr 24, 2023
CI Reliability 2023-04-24 nodejs/reliability#545
Open
CI Reliability 2023-04-25 nodejs/reliability#546
Open
CI Reliability 2023-04-26 nodejs/reliability#547
Open

github-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 2, 2023

VoltrexKeyva added the commit-queue Add this label to land a pull request using GitHub Actions. label May 2, 2023

nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels May 2, 2023

cclauss self-requested a review May 2, 2023 19:06

cclauss approved these changes May 2, 2023
View reviewed changes

VoltrexKeyva added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels May 2, 2023

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label May 2, 2023

nodejs-github-bot merged commit d2156f1 into nodejs:mainMay 2, 2023

VoltrexKeyva deleted the use-pathlib branch May 2, 2023 20:00

github-actionsBot mentioned this pull request May 3, 2023
CI Reliability 2023-05-03 nodejs/reliability#554
Open
33 tasks

targos mentioned this pull request May 3, 2023
v20.1.0 release proposal #47820
Merged

This was referenced May 4, 2023
CI Reliability 2023-05-04 nodejs/reliability#555
Open
CI Reliability 2023-05-05 nodejs/reliability#556
Open

danielleadams mentioned this pull request Jul 10, 2023
v18.17.0 release proposal #48694
Merged

srl295 reviewed Oct 1, 2024
View reviewed changes

srl295 added the i18n-api Issues and PRs related to the i18n implementation. label Oct 1, 2024

srl295 mentioned this pull request Oct 1, 2024
build: configure path refactoring broke ICU download on windows #55214
Closed

Uh oh!

Uh oh!

Conversation

VoltrexKeyva commented Apr 16, 2023

Uh oh!

Uh oh!

targos commented Apr 17, 2023

Uh oh!

Uh oh!

nodejs-github-bot commented Apr 18, 2023

Uh oh!

nodejs-github-bot commented Apr 21, 2023

Uh oh!

nodejs-github-bot commented Apr 21, 2023

Uh oh!

nodejs-github-bot commented Apr 21, 2023

Uh oh!

nodejs-github-bot commented Apr 21, 2023

Uh oh!

cclauss commented Apr 22, 2023

Uh oh!

nodejs-github-bot commented May 2, 2023

Uh oh!

VoltrexKeyva commented May 2, 2023

Uh oh!

cclauss commented May 2, 2023

Uh oh!

nodejs-github-bot commented May 2, 2023

Uh oh!

VoltrexKeyva commented May 2, 2023

Uh oh!

nodejs-github-bot commented May 2, 2023

Uh oh!

cclauss commented May 2, 2023

Uh oh!

srl295Oct 1, 2024

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants