docs: add note about fs.rmdir()/fs.rmdirSync() #14323

al-k21 · 2017-07-17T09:16:18Z

fs.rmdir()/fs.rmdirSync() on the file (not directory) results in different errors on
Windows to everything else

fixes: #8797

Checklist

documentation is changed or added
commit message follows commit guidelines

Affected core subsystem(s)

doc

vsemozhetbyt · 2017-07-17T09:20:34Z

doc/api/fs.md

@@ -2004,6 +2004,9 @@ changes:
 Asynchronous rmdir(2). No arguments other than a possible exception are given
 to the completion callback.

+*Note*: Using `fs.rmdir()` on the file (not directory) results in an `ENOENT`


Thank you!
I am not sure, but maybe "on a file"?

gireeshpunathil · 2017-07-17T10:03:05Z

doc/api/fs.md

@@ -2004,6 +2004,9 @@ changes:
 Asynchronous rmdir(2). No arguments other than a possible exception are given
 to the completion callback.

+*Note*: Using `fs.rmdir()` on a file (not directory) results in an `ENOENT`
+error on Windows and `ENOTDIR` error on Linux/macOS.
+


thanks, also it is a good idea to generalize ENOTDIR for UNIX systems, not just Linux.

Trott · 2017-07-17T11:12:38Z

doc/api/fs.md

@@ -2004,6 +2004,9 @@ changes:
 Asynchronous rmdir(2). No arguments other than a possible exception are given
 to the completion callback.

+*Note*: Using `fs.rmdir()` on a file (not directory) results in an `ENOENT`
+error on Windows and `ENOTDIR` error on UNIX/macOS.


Nit: I know you already changed it from Linux to UNIX, but I believe that throughout or docs, we use POSIX (and that includes macOS, or at least it does in our docs).

/cc @nodejs/documentation in case I'm wrong about some aspect of this. :-D

This should be in Sync version as well, right?

gdams

one small nit and then LGTM

gdams · 2017-07-17T19:48:36Z

doc/api/fs.md

@@ -2004,6 +2004,9 @@ changes:
 Asynchronous rmdir(2). No arguments other than a possible exception are given
 to the completion callback.

+*Note*: Using `fs.rmdir()` on a file (not directory) results in an `ENOENT`


nit not directory => not a directory

gdams · 2017-07-17T19:48:44Z

doc/api/fs.md

@@ -2018,6 +2021,9 @@ changes:

 Synchronous rmdir(2). Returns `undefined`.

+*Note*: Using `fs.rmdirSync()` on a file (not directory) results in an `ENOENT`


nit not directory => not a directory

gibfahn · 2017-07-18T05:27:37Z

doc/api/fs.md

@@ -2004,6 +2004,9 @@ changes:
 Asynchronous rmdir(2). No arguments other than a possible exception are given
 to the completion callback.

+*Note*: Using `fs.rmdir()` on a file (not a directory) results in an `ENOENT`
+error on Windows and `ENOTDIR` error on POSIX.


ENOTDIR error -> an ENOTDIR error (same below)

fs.rmdir() on the file (not directory) results in different errors on Windows to everything else fixes: nodejs#8797

gdams

LGTM

fs.rmdir() on the file (not directory) results in different errors on Windows to everything else Fixes: #8797 PR-URL: #14323 Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>

jasnell · 2017-07-19T21:13:14Z

Landed in bd7735e

gibfahn · 2017-07-20T06:45:27Z

Congrats on becoming a Contributor @al-k21 !

gdams · 2017-07-22T10:58:56Z

Nice work @al-k21

fs.rmdir() on the file (not directory) results in different errors on Windows to everything else Fixes: #8797 PR-URL: #14323 Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>

nodejs-github-bot added doc Issues and PRs related to the documentations. fs Issues and PRs related to the fs subsystem / file system. labels Jul 17, 2017

vsemozhetbyt reviewed Jul 17, 2017

View reviewed changes

al-k21 force-pushed the docs branch from d377bec to e96e0d3 Compare July 17, 2017 09:26

vsemozhetbyt approved these changes Jul 17, 2017

View reviewed changes

gireeshpunathil requested changes Jul 17, 2017

View reviewed changes

al-k21 force-pushed the docs branch from e96e0d3 to 82d0662 Compare July 17, 2017 10:19

Trott reviewed Jul 17, 2017

View reviewed changes

al-k21 force-pushed the docs branch from 82d0662 to 7a43484 Compare July 17, 2017 12:15

al-k21 changed the title ~~docs: add note about fs.rmdir()~~ docs: add note about fs.rmdir()/fs.rmdirSync() Jul 17, 2017

gireeshpunathil approved these changes Jul 17, 2017

View reviewed changes

lpinca approved these changes Jul 17, 2017

View reviewed changes

jasnell approved these changes Jul 17, 2017

View reviewed changes

Fishrock123 approved these changes Jul 17, 2017

View reviewed changes

gdams suggested changes Jul 17, 2017

View reviewed changes

cjihrig approved these changes Jul 17, 2017

View reviewed changes

al-k21 force-pushed the docs branch from 7a43484 to 7491dc7 Compare July 18, 2017 05:12

gibfahn reviewed Jul 18, 2017

View reviewed changes

docs: add note about fs.rmdir()

6489a41

fs.rmdir() on the file (not directory) results in different errors on Windows to everything else fixes: nodejs#8797

al-k21 force-pushed the docs branch from 7491dc7 to 6489a41 Compare July 18, 2017 07:57

gdams approved these changes Jul 18, 2017

View reviewed changes

gibfahn approved these changes Jul 18, 2017

View reviewed changes

jasnell closed this Jul 19, 2017

addaleax mentioned this pull request Jul 24, 2017

v8.3.0 proposal #14452

Closed

addaleax mentioned this pull request Aug 2, 2017

v8.3.0 proposal #14594

Merged

MylesBorins added the land-on-v6.x label Aug 16, 2017

MylesBorins mentioned this pull request Aug 16, 2017

v6.11.3-proposal #14852

Merged

al-k21 deleted the docs branch October 18, 2017 13:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs: add note about fs.rmdir()/fs.rmdirSync() #14323

docs: add note about fs.rmdir()/fs.rmdirSync() #14323

al-k21 commented Jul 17, 2017 •

edited

Loading

vsemozhetbyt Jul 17, 2017

gireeshpunathil Jul 17, 2017

Trott Jul 17, 2017

thefourtheye Jul 17, 2017

gdams left a comment

gdams Jul 17, 2017

gdams Jul 17, 2017

gibfahn Jul 18, 2017 •

edited

Loading

gdams left a comment

jasnell commented Jul 19, 2017

gibfahn commented Jul 20, 2017

gdams commented Jul 22, 2017

		@@ -2018,6 +2021,9 @@ changes:

		Synchronous rmdir(2). Returns `undefined`.

		Note: Using `fs.rmdirSync()` on a file (not directory) results in an `ENOENT`

docs: add note about fs.rmdir()/fs.rmdirSync() #14323

docs: add note about fs.rmdir()/fs.rmdirSync() #14323

Conversation

al-k21 commented Jul 17, 2017 • edited Loading

Checklist

Affected core subsystem(s)

vsemozhetbyt Jul 17, 2017

Choose a reason for hiding this comment

gireeshpunathil Jul 17, 2017

Choose a reason for hiding this comment

Trott Jul 17, 2017

Choose a reason for hiding this comment

thefourtheye Jul 17, 2017

Choose a reason for hiding this comment

gdams left a comment

Choose a reason for hiding this comment

gdams Jul 17, 2017

Choose a reason for hiding this comment

gdams Jul 17, 2017

Choose a reason for hiding this comment

gibfahn Jul 18, 2017 • edited Loading

Choose a reason for hiding this comment

gdams left a comment

Choose a reason for hiding this comment

jasnell commented Jul 19, 2017

gibfahn commented Jul 20, 2017

gdams commented Jul 22, 2017

al-k21 commented Jul 17, 2017 •

edited

Loading

gibfahn Jul 18, 2017 •

edited

Loading