Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

module: fix stat with long paths on windows #2011

Closed
wants to merge 1 commit into from
Closed

module: fix stat with long paths on windows #2011

wants to merge 1 commit into from

Conversation

Fishrock123
Copy link
Contributor

See https://github.com/nodejs/io.js/pull/1920/files#r32701699, this also requires the long path fix from #1991

@Fishrock123 Fishrock123 added the module Issues and PRs related to the module subsystem. label Jun 18, 2015
@Fishrock123 Fishrock123 mentioned this pull request Jun 18, 2015
Closed
@bnoordhuis
Copy link
Member

LGTM but a regression test would be nice.

@targos
Copy link
Member

targos commented Jun 18, 2015

this test I added in #1991 may be enough. I can try when I am back home.

@targos
Copy link
Member

targos commented Jun 18, 2015

or start a CI on master ? It will fail if I am right

@ChALkeR
Copy link
Member

ChALkeR commented Jun 18, 2015

I still think that it might be better to wrap internalModuleStat and internalModuleReadFile…

@Fishrock123
Copy link
Contributor Author

CI on master: https://jenkins-iojs.nodesource.com/view/iojs/job/iojs+any-pr+multi/47/

@targos
Copy link
Member

targos commented Jun 18, 2015

@ChALkeR I agree. We can introduce internal/fs to expose the wrapped functions

@mscdex mscdex added the windows Issues and PRs related to the Windows platform. label Jun 18, 2015
@Fishrock123
Copy link
Contributor Author

CI on master does not appear to catch it?

@ChALkeR ChALkeR mentioned this pull request Jun 18, 2015
Closed
@targos
Copy link
Member

targos commented Jun 18, 2015

apparently not 😞

@vkurchatkin
Copy link
Contributor

I still think that it might be better to wrap internalModuleStat and internalModuleReadFile…

+1

@targos targos mentioned this pull request Jun 18, 2015
Closed
@Fishrock123
Copy link
Contributor Author

#2013

@gr00nd gr00nd mentioned this pull request Feb 6, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
module Issues and PRs related to the module subsystem. windows Issues and PRs related to the Windows platform.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@Fishrock123 @bnoordhuis @targos @ChALkeR @vkurchatkin @mscdex