Skip to content

Conversation

@TimothyGu
Copy link
Member

First commit (#14438):

commit 5180f932eaccfa380f68908b00b098205cb466ba Author: Tobias Nießen <[email protected]> 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 <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Timothy Gu <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Khaidi Chu <[email protected]> 

Second commit (#14440):

commit 6fa5f3575422a12d374b349e3234331ca5e734c8 Author: Timothy Gu <[email protected]> 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 <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> 
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

tniessenand others added 2 commits August 12, 2017 17:35
PR-URL: nodejs#14438 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Timothy Gu <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Khaidi Chu <[email protected]>
`path.resolve()` and `path.join()` are left alone in this commit due to the lack of clear semantics. PR-URL: nodejs#14440Fixes: nodejs#14405 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
@nodejs-github-botnodejs-github-bot added path Issues and PRs related to the path subsystem. v6.x labels Aug 12, 2017
@TimothyGu
Copy link
MemberAuthor

@tniessen
Copy link
Member

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

@TimothyGu
Copy link
MemberAuthor

@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 <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Timothy Gu <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Khaidi Chu <[email protected]>
@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 <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Timothy Gu <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Khaidi Chu <[email protected]>
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: #14440Fixes: #14405 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
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 <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Timothy Gu <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Khaidi Chu <[email protected]>
@MylesBorins
Copy link
Contributor

landed in ee98df8...d75363b

@TimothyGuTimothyGu 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 <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Timothy Gu <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Khaidi Chu <[email protected]>
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: #14440Fixes: #14405 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
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 <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Timothy Gu <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Khaidi Chu <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pathIssues and PRs related to the path subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@TimothyGu@tniessen@MylesBorins@refack@nodejs-github-bot