Skip to content

doc: add note for fs.watch on virtualized env#6809

Closed
eljefedelrodeodeljefe wants to merge 1 commit intonodejs:masterfrom
eljefedelrodeodeljefe:doc/fs-watch-virtuallization
Closed

doc: add note for fs.watch on virtualized env#6809
eljefedelrodeodeljefe wants to merge 1 commit intonodejs:masterfrom
eljefedelrodeodeljefe:doc/fs-watch-virtuallization

Conversation

@eljefedelrodeodeljefe
Copy link
Contributor

@eljefedelrodeodeljefe eljefedelrodeodeljefe commented May 17, 2016

Checklist
  • tests and code linting passes
  • a test and/or benchmark is included
  • documentation is changed or added
  • the commit message follows commit guidelines
Affected core subsystem(s)

doc

Description of change

On virtualization systems fs.watch doesn't work reliably, which is implicitly already in the respective docs. To make it more explicit this PR adds a subsentence. The following statement defers to use of fs.watchFile, which is the potential workaround.

Fixes: #6765

@eljefedelrodeodeljefe eljefedelrodeodeljefe added doc Issues and PRs related to the documentations. fs Issues and PRs related to the fs subsystem / file system. labels May 17, 2016
@eljefedelrodeodeljefe eljefedelrodeodeljefe mentioned this pull request May 17, 2016
Closed
@cjihrig
Copy link
Contributor

cjihrig commented May 17, 2016

LGTM

1 similar comment
@thefourtheye
Copy link
Contributor

thefourtheye commented May 17, 2016

LGTM

jasnell
jasnell reviewed May 17, 2016
View reviewed changes
doc/api/fs.md Outdated
Copy link
Member

@jasnell jasnell May 17, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

minor nit... I'd restructure the sentence just a bit...

Something like:

For example, watching files or directories can be unreliable, and in some
cases impossible, on network file systems (NFS, SMB, etc), and on host
file systems when using virtualization software such as Vagrant, Docker, etc.

But I'm good with this also ;-)

@jasnell
Copy link
Member

jasnell commented May 17, 2016

Left one comment but generally LGTM

@eljefedelrodeodeljefe
Copy link
Contributor Author

eljefedelrodeodeljefe commented May 17, 2016

Fine for me. Will adapt it before landing. Thx

@mhdawson
Copy link
Member

mhdawson commented May 17, 2016

I'm + 1for @jasnell's version, but generally LGTM as well.

bnoordhuis
bnoordhuis reviewed May 18, 2016
View reviewed changes
doc/api/fs.md Outdated
Copy link
Member

@bnoordhuis bnoordhuis May 18, 2016

Choose a reason for hiding this comment

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

s/and on/or/? I suspect most programmers are like me and interpret it boolean logic-style.

@bnoordhuis
Copy link
Member

bnoordhuis commented May 18, 2016

LGTM with a suggestion.

@eljefedelrodeodeljefe
doc: add note for fs.watch virtualized env
3d5fe13 
Fixes: #6765
PR-URL: #6809
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
eljefedelrodeodeljefe added a commit that referenced this pull request May 19, 2016
@eljefedelrodeodeljefe
doc: add note for fs.watch virtualized env
ae0f68d 
Fixes: #6765
PR-URL: #6809
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
@eljefedelrodeodeljefe
Copy link
Contributor Author

eljefedelrodeodeljefe commented May 19, 2016

Went ahead and landed it in ae0f68d

Addressed both @bnoordhuis an @jasnell nits

For example, watching files or
directories can be unreliable, and in some cases impossible, on network file
systems (NFS, SMB, etc), or host file systems when using virtualization software
such as Vagrant, Docker, etc.

Fishrock123 pushed a commit that referenced this pull request May 23, 2016
@eljefedelrodeodeljefe @Fishrock123
doc: add note for fs.watch virtualized env
6d71cbc 
Fixes: #6765
PR-URL: #6809
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
rvagg pushed a commit that referenced this pull request Jun 2, 2016
@eljefedelrodeodeljefe @rvagg
doc: add note for fs.watch virtualized env
b52d838 
Fixes: #6765
PR-URL: #6809
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
@MylesBorins MylesBorins mentioned this pull request Jun 2, 2016
Closed
MylesBorins pushed a commit that referenced this pull request Jun 3, 2016
@eljefedelrodeodeljefe
doc: add note for fs.watch virtualized env
ccae698 
Fixes: #6765
PR-URL: #6809
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
@MylesBorins MylesBorins mentioned this pull request Jun 24, 2016
Merged
MylesBorins pushed a commit that referenced this pull request Jun 24, 2016
@eljefedelrodeodeljefe
doc: add note for fs.watch virtualized env
2b0f40c 
Fixes: #6765
PR-URL: #6809
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
MylesBorins pushed a commit that referenced this pull request Jun 24, 2016
@eljefedelrodeodeljefe
doc: add note for fs.watch virtualized env
062009c 
Fixes: #6765
PR-URL: #6809
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
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

doc Issues and PRs related to the documentations. fs Issues and PRs related to the fs subsystem / file system.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@eljefedelrodeodeljefe @cjihrig @thefourtheye @jasnell @mhdawson @bnoordhuis @MylesBorins