Skip to content

Conversation

@TimothyGu
Copy link
Member

First commit (#14438):

commit 5180f932eaccfa380f68908b00b098205cb466ba Author: Tobias Nießen <tniessen@tnie.de> Date: Mon Jul 24 16:20:30 2017 +0200 test: increase coverage for path.parse PR-URL: https://github.com/nodejs/node/pull/14438 Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Timothy Gu <timothygu99@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Khaidi Chu <i@2333.moe> 

Second commit (#14440):

commit 6fa5f3575422a12d374b349e3234331ca5e734c8 Author: Timothy Gu <timothygu99@gmail.com> Date: Mon Jul 24 08:39:02 2017 +0800 path: fix win32 volume-relative paths `path.resolve()` and `path.join()` are left alone in this commit due to the lack of clear semantics. PR-URL: https://github.com/nodejs/node/pull/14440 Fixes: https://github.com/nodejs/node/issues/14405 Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> 
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

path

tniessen and others added 2 commits August 12, 2017 17:35
PR-URL: nodejs#14438 Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Timothy Gu <timothygu99@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Khaidi Chu <i@2333.moe>
`path.resolve()` and `path.join()` are left alone in this commit due to the lack of clear semantics. PR-URL: nodejs#14440 Fixes: nodejs#14405 Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de>
@TimothyGu TimothyGu requested review from refack and tniessen August 12, 2017 10:30
@nodejs-github-bot nodejs-github-bot added path Issues and PRs related to the path subsystem. v6.x labels Aug 12, 2017
@tniessen
Copy link
Member

Did you intentionally not backport 2ac0aff (part of #14438)?

@TimothyGu
Copy link
Member Author

@tniessen No it was not intentional. I backported the test commit in #14438 because it caused the merge conflict in #14440 but I'll backport the other one too. Thanks for noticing!

As the length of `path` is known at this point, there is no point in making an exact copy using `slice`. PR-URL: nodejs#14438 Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Timothy Gu <timothygu99@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Khaidi Chu <i@2333.moe>
@tniessen
Copy link
Member

I pushed the missing commit as discussed on IRC.

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

MylesBorins pushed a commit that referenced this pull request Aug 14, 2017
Backport-PR-URL: #14787 PR-URL: #14438 Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Timothy Gu <timothygu99@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Khaidi Chu <i@2333.moe>
MylesBorins pushed a commit that referenced this pull request Aug 14, 2017
`path.resolve()` and `path.join()` are left alone in this commit due to the lack of clear semantics. Backport-PR-URL: #14787 PR-URL: #14440 Fixes: #14405 Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de>
MylesBorins pushed a commit that referenced this pull request Aug 14, 2017
As the length of `path` is known at this point, there is no point in making an exact copy using `slice`. Backport-PR-URL: #14787 PR-URL: #14438 Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Timothy Gu <timothygu99@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Khaidi Chu <i@2333.moe>
@MylesBorins
Copy link
Contributor

landed in ee98df8...d75363b

@TimothyGu TimothyGu deleted the v6.x-14440 branch August 15, 2017 06:30
MylesBorins pushed a commit that referenced this pull request Aug 16, 2017
Backport-PR-URL: #14787 PR-URL: #14438 Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Timothy Gu <timothygu99@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Khaidi Chu <i@2333.moe>
MylesBorins pushed a commit that referenced this pull request Aug 16, 2017
`path.resolve()` and `path.join()` are left alone in this commit due to the lack of clear semantics. Backport-PR-URL: #14787 PR-URL: #14440 Fixes: #14405 Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de>
MylesBorins pushed a commit that referenced this pull request Aug 16, 2017
As the length of `path` is known at this point, there is no point in making an exact copy using `slice`. Backport-PR-URL: #14787 PR-URL: #14438 Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Timothy Gu <timothygu99@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Khaidi Chu <i@2333.moe>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

path Issues and PRs related to the path subsystem.

5 participants