Skip to content

path: add path/posix and path/win32 alias modules - #34962

Closed
ExE-Boss wants to merge 3 commits into
nodejs:masterfrom
ExE-Boss:lib/path/add-path-posix-win-alias-modules
Closed

path: add path/posix and path/win32 alias modules#34962
ExE-Boss wants to merge 3 commits into
nodejs:masterfrom
ExE-Boss:lib/path/add-path-posix-win-alias-modules

Conversation

@ExE-Boss

@ExE-BossExE-Boss commented Aug 28, 2020

Copy link
Copy Markdown
Contributor

Turns out, #34055 is not the last alias module, because I forgot about path.posix and path.win32.


Refs:#31553
Refs:#32953
Refs:#33950
Refs:#34001
Refs:#34002
Refs:#34055

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-botnodejs-github-bot added the build Issues and PRs related to build files or the CI. label Aug 28, 2020
@ExE-BossExE-Boss mentioned this pull request Aug 28, 2020
4 tasks
@richardlaurichardlau added the semver-major PRs that contain breaking changes and should be released in the next major version. label Sep 29, 2020
@ExE-Boss
ExE-Bossforce-pushed the lib/path/add-path-posix-win-alias-modules branch from 440f191 to 4c75a27CompareOctober 6, 2020 16:17

@MylesBorinsMylesBorins left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

RSLGTM

@ExE-Boss

Copy link
Copy Markdown
ContributorAuthor

review?(@jasnell, @mcollina, @targos, @tniessen, @Trott)

Comment threadlib/path/posix.js

@mcollinamcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

@ExE-Boss
ExE-Boss requested a review from ljharbOctober 14, 2020 12:21
@ExE-Boss

Copy link
Copy Markdown
ContributorAuthor

@jasnell@MylesBorins@Trott@targos@BridgeAR

It’s only a few days left until v15.0.0 ships, and I’d like to get this merged before then.

@Trott

Copy link
Copy Markdown
Member

Repeating the comment from #34055 here as well:

We're a few weeks past the semver-major cut-off date for 15.x.x so this would probably require assent from both @nodejs/tsc and Releasers (and in particular @BethGriggs who is preparing the release) to land in 15.x.x.

@richardlau Is this semver major because it might have a naming clash with existing modules? Or is there another reason?

@richardlau

Copy link
Copy Markdown
Member

Repeating the comment from #34055 here as well:

We're a few weeks past the semver-major cut-off date for 15.x.x so this would probably require assent from both @nodejs/tsc and Releasers (and in particular @BethGriggs who is preparing the release) to land in 15.x.x.

@richardlau Is this semver major because it might have a naming clash with existing modules? Or is there another reason?

I was under the impression that new modules were major due to naming clashes (and remember that "existing modules" include things that are not on npm's registry, e.g. local files). I think I did see someone post in an issue that that isn't what is writing down so 🤷.

@BethGriggs

Copy link
Copy Markdown
Member

We're a few weeks past the semver-major cut-off date for 15.x.x so this would probably require assent from both @nodejs/tsc and Releasers (and in particular @BethGriggs who is preparing the release) to land in 15.x.x.

It would probably need to land in the next few hours to make it into v15.0.0 as I don't think we should make changes < 24 hours until the release (unless critical).

@TrottTrott added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 19, 2020
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 19, 2020
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@Trott

Copy link
Copy Markdown
Member

Comment threaddoc/api/path.md Outdated
Co-authored-by: Rich Trott <rtrott@gmail.com>
Comment threaddoc/api/path.md Outdated
@Trott

Copy link
Copy Markdown
Member

I'm going to be offline for most of the day, but if someone else can try to shepherd this through, that would be great. CITGM spot check looks good to me, so it's really just a matter of resolving whatever lint issues might remain, getting a green CI, and landing.

Co-authored-by: Rich Trott <rtrott@gmail.com>
@codecov-io

Copy link
Copy Markdown

Codecov Report

Merging #34962 into master will decrease coverage by 0.47%.
The diff coverage is 100.00%.

Impacted file tree graph

@@ Coverage Diff @@## master #34962 +/- ##
==========================================
- Coverage 96.87% 96.40% -0.48% 
==========================================
Files 212 222 +10 Lines 69609 73681 +4072 ==========================================
+ Hits 67435 71033 +3598 - Misses 2174 2648 +474 
Impacted FilesCoverage Δ
lib/_http_client.js98.45% <100.00%> (+0.01%)⬆️
lib/assert.js98.09% <100.00%> (+<0.01%)⬆️
lib/events.js99.27% <100.00%> (+<0.01%)⬆️
lib/fs.js94.13% <100.00%> (-0.14%)⬇️
lib/internal/assert/assertion_error.js100.00% <100.00%> (ø)
lib/internal/bootstrap/loaders.js97.18% <100.00%> (+0.01%)⬆️
lib/internal/buffer.js100.00% <100.00%> (ø)
lib/internal/errors.js97.97% <100.00%> (+<0.01%)⬆️
lib/internal/event_target.js97.04% <100.00%> (+<0.01%)⬆️
lib/internal/fs/read_file_context.js98.31% <100.00%> (ø)
... and 80 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8beef5e...c980bd8. Read the comment docs.

nodejs-github-bot pushed a commit that referenced this pull request Oct 20, 2020
Refs: #31553
Refs: #32953
Refs: #33950
Refs: #34001
Refs: #34002
Refs: #34055
PR-URL: #34962
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
@ljharb

Copy link
Copy Markdown
Member

v15 has already been cut before landing this, which means master is now v16?

@ExE-Boss
ExE-Boss deleted the lib/path/add-path-posix-win-alias-modules branch October 20, 2020 19:38
aduh95 pushed a commit that referenced this pull request Oct 20, 2020
Refs: #31553
Refs: #32953
Refs: #33950
Refs: #34001
Refs: #34002
PR-URL: #34055
Refs: #34962
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
ExE-Boss added a commit to ExE-Boss/node that referenced this pull request Oct 24, 2020
Fixes: nodejs#35740
Refs: nodejs#31553
Refs: nodejs#32953
Refs: nodejs#33991
Refs: nodejs#34001
Refs: nodejs#34055
Refs: nodejs#34962
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
Co-authored-by: Trivikram Kamat <trivikr.dev@gmail.com>
Co-authored-by: Rich Trott <rtrott@gmail.com>
nodejs-github-bot pushed a commit that referenced this pull request Oct 25, 2020
Fixes: #35740
Refs: #31553
Refs: #32953
Refs: #33991
Refs: #34001
Refs: #34055
Refs: #34962
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
Co-authored-by: Trivikram Kamat <trivikr.dev@gmail.com>
Co-authored-by: Rich Trott <rtrott@gmail.com>
PR-URL: #34002
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Masashi Hirano <shisama07@gmail.com>
Reviewed-By: Pranshu Srivastava <rexagod@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
targos pushed a commit that referenced this pull request Nov 3, 2020
Fixes: #35740
Refs: #31553
Refs: #32953
Refs: #33991
Refs: #34001
Refs: #34055
Refs: #34962
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
Co-authored-by: Trivikram Kamat <trivikr.dev@gmail.com>
Co-authored-by: Rich Trott <rtrott@gmail.com>
PR-URL: #34002
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Masashi Hirano <shisama07@gmail.com>
Reviewed-By: Pranshu Srivastava <rexagod@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
@targostargos added semver-minor PRs that contain new features and should be released in the next minor version. and removed semver-major PRs that contain breaking changes and should be released in the next major version. labels Nov 11, 2020
codebytere pushed a commit that referenced this pull request Nov 22, 2020
Refs: #31553
Refs: #32953
Refs: #33950
Refs: #34001
Refs: #34002
Refs: #34055
PR-URL: #34962
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
codebytere pushed a commit that referenced this pull request Nov 22, 2020
Refs: #31553
Refs: #32953
Refs: #33950
Refs: #34001
Refs: #34002
PR-URL: #34055
Refs: #34962
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
codebytere added a commit that referenced this pull request Nov 22, 2020
Notable changes:
dns:
* (SEMVER-MINOR) add a cancel() method to the promise Resolver (Szymon Marczak) #33099
events:
* (SEMVER-MINOR) add max listener warning for EventTarget (James M Snell) #36001
http:
* (SEMVER-MINOR) add support for abortsignal to http.request (Benjamin Gruenbaum) #36048
http2:
* (SEMVER-MINOR) allow setting the local window size of a session (Yongsheng Zhang) #35978
lib:
* (SEMVER-MINOR) add throws option to fs.f/l/statSync (Andrew Casey) #33716
path:
* (SEMVER-MINOR) add `path/posix` and `path/win32` alias modules (ExE Boss) #34962
readline:
* (SEMVER-MINOR) add getPrompt to get the current prompt (Mattias Runge-Broberg) #33675
src:
* (SEMVER-MINOR) add loop idle time in diagnostic report (Gireesh Punathil) #35940
util:
* (SEMVER-MINOR) add `util/types` alias module (ExE Boss) #34055
PR-URL: TODO
@codebyterecodebytere mentioned this pull request Nov 22, 2020
codebytere added a commit that referenced this pull request Nov 24, 2020
Notable changes:
dns:
* (SEMVER-MINOR) add a cancel() method to the promise Resolver (Szymon Marczak) #33099
events:
* (SEMVER-MINOR) add max listener warning for EventTarget (James M Snell) #36001
http:
* (SEMVER-MINOR) add support for abortsignal to http.request (Benjamin Gruenbaum) #36048
http2:
* (SEMVER-MINOR) allow setting the local window size of a session (Yongsheng Zhang) #35978
lib:
* (SEMVER-MINOR) add throws option to fs.f/l/statSync (Andrew Casey) #33716
path:
* (SEMVER-MINOR) add `path/posix` and `path/win32` alias modules (ExE Boss) #34962
readline:
* (SEMVER-MINOR) add getPrompt to get the current prompt (Mattias Runge-Broberg) #33675
src:
* (SEMVER-MINOR) add loop idle time in diagnostic report (Gireesh Punathil) #35940
util:
* (SEMVER-MINOR) add `util/types` alias module (ExE Boss) #34055
PR-URL: TODO
codebytere added a commit that referenced this pull request Nov 24, 2020
Notable changes:
dns:
* (SEMVER-MINOR) add a cancel() method to the promise Resolver (Szymon Marczak) #33099
events:
* (SEMVER-MINOR) add max listener warning for EventTarget (James M Snell) #36001
http:
* (SEMVER-MINOR) add support for abortsignal to http.request (Benjamin Gruenbaum) #36048
http2:
* (SEMVER-MINOR) allow setting the local window size of a session (Yongsheng Zhang) #35978
lib:
* (SEMVER-MINOR) add throws option to fs.f/l/statSync (Andrew Casey) #33716
path:
* (SEMVER-MINOR) add `path/posix` and `path/win32` alias modules (ExE Boss) #34962
readline:
* (SEMVER-MINOR) add getPrompt to get the current prompt (Mattias Runge-Broberg) #33675
src:
* (SEMVER-MINOR) add loop idle time in diagnostic report (Gireesh Punathil) #35940
util:
* (SEMVER-MINOR) add `util/types` alias module (ExE Boss) #34055
PR-URL: #36232
codebytere added a commit that referenced this pull request Nov 24, 2020
Notable changes:
dns:
* (SEMVER-MINOR) add a cancel() method to the promise Resolver (Szymon Marczak) #33099
events:
* (SEMVER-MINOR) add max listener warning for EventTarget (James M Snell) #36001
http:
* (SEMVER-MINOR) add support for abortsignal to http.request (Benjamin Gruenbaum) #36048
http2:
* (SEMVER-MINOR) allow setting the local window size of a session (Yongsheng Zhang) #35978
lib:
* (SEMVER-MINOR) add throws option to fs.f/l/statSync (Andrew Casey) #33716
path:
* (SEMVER-MINOR) add `path/posix` and `path/win32` alias modules (ExE Boss) #34962
readline:
* (SEMVER-MINOR) add getPrompt to get the current prompt (Mattias Runge-Broberg) #33675
src:
* (SEMVER-MINOR) add loop idle time in diagnostic report (Gireesh Punathil) #35940
util:
* (SEMVER-MINOR) add `util/types` alias module (ExE Boss) #34055
PR-URL: #36232
targos pushed a commit that referenced this pull request May 16, 2021
Fixes: #35740
Refs: #31553
Refs: #32953
Refs: #33991
Refs: #34001
Refs: #34055
Refs: #34962
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
Co-authored-by: Trivikram Kamat <trivikr.dev@gmail.com>
Co-authored-by: Rich Trott <rtrott@gmail.com>
PR-URL: #34002
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Masashi Hirano <shisama07@gmail.com>
Reviewed-By: Pranshu Srivastava <rexagod@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
targos pushed a commit that referenced this pull request Jun 11, 2021
Fixes: #35740
Refs: #31553
Refs: #32953
Refs: #33991
Refs: #34001
Refs: #34055
Refs: #34962
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
Co-authored-by: Trivikram Kamat <trivikr.dev@gmail.com>
Co-authored-by: Rich Trott <rtrott@gmail.com>
PR-URL: #34002
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Masashi Hirano <shisama07@gmail.com>
Reviewed-By: Pranshu Srivastava <rexagod@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.buildIssues and PRs related to build files or the CI.semver-minorPRs that contain new features and should be released in the next minor version.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

13 participants

@ExE-Boss@Trott@richardlau@BethGriggs@nodejs-github-bot@codecov-io@ljharb@mcollina@jasnell@MylesBorins@BridgeAR@Flarna@targos