Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34.4k
doc: add note for fs.watch on virtualized env#6809
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.
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
doc: add note for fs.watch on virtualized env #6809
Uh oh!
There was an error while loading. Please reload this page.
Conversation
cjihrig commented May 17, 2016
LGTM |
1 similar comment
thefourtheye commented May 17, 2016
LGTM |
doc/api/fs.md Outdated
jasnellMay 17, 2016 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
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 commented May 17, 2016
Left one comment but generally LGTM |
eljefedelrodeodeljefe commented May 17, 2016
Fine for me. Will adapt it before landing. Thx |
mhdawson commented May 17, 2016
I'm + 1for @jasnell's version, but generally LGTM as well. |
doc/api/fs.md Outdated
There was a problem hiding this comment.
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 commented May 18, 2016
LGTM with a suggestion. |
Fixes: #6765 PR-URL: #6809 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
Fixes: #6765 PR-URL: #6809 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
eljefedelrodeodeljefe commented May 19, 2016
Went ahead and landed it in ae0f68d Addressed both @bnoordhuis an @jasnell nits |
Fixes: #6765 PR-URL: #6809 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
Fixes: #6765 PR-URL: #6809 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
Fixes: #6765 PR-URL: #6809 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
Fixes: #6765 PR-URL: #6809 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
Fixes: #6765 PR-URL: #6809 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
Checklist
Affected core subsystem(s)
docDescription of change
On virtualization systems
fs.watchdoesn'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