FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

doc: fs.mkdir('/') throws EPERM on Windows by coreyfarrell · Pull Request #25340 · nodejs/node · GitHub

/ node Public

doc: fs.mkdir('/') throws EPERM on Windows - #25340

Closed
coreyfarrell wants to merge 1 commit into
nodejs:masterfrom
coreyfarrell:fs-mkdir-docs
ConversationCommits (1)ChecksFiles changed
Closed

doc: fs.mkdir('/') throws EPERM on Windows#25340
coreyfarrell wants to merge 1 commit into
nodejs:masterfrom
coreyfarrell:fs-mkdir-docs

Conversation

coreyfarrell commented Jan 4, 2019

Copy link
Copy Markdown
Member

Fixes: #25110

Checklist

CC @bcoe @nodejs/fs

nodejs-github-bot added doc Issues and PRs related to the documentations. fs Issues and PRs related to the fs subsystem / file system. labels Jan 4, 2019
Trott requested changes Jan 4, 2019
View reviewed changes

Trott 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Hi, @coreyfarrell! Welcome and thanks for the pull request.

I've left a comment with a requested change. It wraps the text at 80 characters, which you should have received an error about if you ran make lint-md (fast) or make test (slow). It also changes the wording, adds punctuation, and styling. Please take a look.

Comment thread doc/api/fs.md Outdated
Trott added the windows Issues and PRs related to the Windows platform. label Jan 4, 2019
coreyfarrell mentioned this pull request Jan 4, 2019
Merged

coreyfarrell commented Jan 4, 2019

Copy link
Copy Markdown
Member Author

@MylesBorins This documentation update is intended for v10.x and v11.x in addition to master.

I assume you don't want me to open PR's for each branch but please let me know if I need to take additional actions or how to handle situations like this in the future.

Trott reviewed Jan 4, 2019
View reviewed changes
Comment thread doc/api/fs.md Outdated
Trott approved these changes Jan 4, 2019
View reviewed changes

Trott 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM with or without the nit-pick suggestion I just left.

Trott commented Jan 4, 2019

Copy link
Copy Markdown
Member

I assume you don't want me to open PR's for each branch

Someone will cherry-pick the commit to the other branches. If the cherry-pick does not work because of a conflict, they may leave a comment here asking you to open a backport PR. There's nothing to do at this time, though.

bcoe approved these changes Jan 4, 2019
View reviewed changes

bcoe 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

thanks for adding this note @coreyfarrell take or leave my comma preferences.

Comment thread doc/api/fs.md
});
```

On Windows, using `fs.mkdir()` on the root directory even with recursion will

bcoe Jan 4, 2019 •
edited
Loading

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

the nit picky editor in me wonders whether this should be:

Using `fs.mkdir()` on the root directory, even with recursion, will
result in an error on Windows:

coreyfarrell Jan 4, 2019

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

My preference is for the fact that it's platform specific to be first but I'm willing to do whatever the consensus is.

Trott Jan 4, 2019

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This maybe?

On Windows, using `fs.mkdir()` on the root directory, even with recursion, will
result in an error:

bcoe added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Jan 6, 2019

bcoe commented Jan 6, 2019

Copy link
Copy Markdown
Contributor

ci: https://ci.nodejs.org/job/node-test-commit/24828/

bcoe pushed a commit that referenced this pull request Jan 6, 2019
doc: fs.mkdir('/') throws EPERM on Windows
e7c6f4f 
Fixes: #25110

PR-URL: #25340
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ben Coe <bencoe@gmail.com>

bcoe commented Jan 6, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor

Landed in e7c6f4f

bcoe closed this Jan 6, 2019

bcoe commented Jan 6, 2019

Copy link
Copy Markdown
Contributor

@coreyfarrell congrats on your first contribution to Node.js 🎉

coreyfarrell deleted the fs-mkdir-docs branch January 6, 2019 22:06
addaleax pushed a commit that referenced this pull request Jan 9, 2019
doc: fs.mkdir('/') throws EPERM on Windows
f8dcbba 
Fixes: #25110

PR-URL: #25340
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ben Coe <bencoe@gmail.com>
refack pushed a commit to refack/node that referenced this pull request Jan 14, 2019
doc: fs.mkdir('/') throws EPERM on Windows
2aa197b 
Fixes: nodejs#25110

PR-URL: nodejs#25340
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ben Coe <bencoe@gmail.com>
BridgeAR mentioned this pull request Jan 16, 2019
Merged
BridgeAR pushed a commit to BridgeAR/node that referenced this pull request Jan 16, 2019
doc: fs.mkdir('/') throws EPERM on Windows
b1023ba 
Fixes: nodejs#25110

PR-URL: nodejs#25340
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ben Coe <bencoe@gmail.com>
MylesBorins mentioned this pull request Jan 24, 2019
Merged
BethGriggs pushed a commit that referenced this pull request Apr 28, 2019
doc: fs.mkdir('/') throws EPERM on Windows
4717767 
Fixes: #25110

PR-URL: #25340
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ben Coe <bencoe@gmail.com>
BethGriggs mentioned this pull request May 1, 2019
Merged
BethGriggs pushed a commit that referenced this pull request May 10, 2019
doc: fs.mkdir('/') throws EPERM on Windows
757cf4d 
Fixes: #25110

PR-URL: #25340
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ben Coe <bencoe@gmail.com>
MylesBorins pushed a commit that referenced this pull request May 16, 2019
doc: fs.mkdir('/') throws EPERM on Windows
ef6e4f1 
Fixes: #25110

PR-URL: #25340
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ben Coe <bencoe@gmail.com>
This was referenced May 29, 2019
Closed
Closed
Closed
Closed
Closed
This was referenced Aug 9, 2020
Open
Open
Open
Open
Open
Open
Merged
snyk-bot mentioned this pull request Oct 29, 2020
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Trott Trott approved these changes
lpinca lpinca approved these changes
+1 more reviewer
bcoe bcoe approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. doc Issues and PRs related to the documentations. fs Issues and PRs related to the fs subsystem / file system. 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.

5 participants

Back | FazBrowse Home | New Git URL