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

doc: added note to fs.watchFile on previousStat #16099

Closed
wants to merge 3 commits into from
Closed

doc: added note to fs.watchFile on previousStat #16099

wants to merge 3 commits into from

Conversation

nikniv
Copy link
Contributor

@nikniv nikniv commented Oct 9, 2017

Checklist:

Fixes: #15364

@nikniv
doc: added note to fs.watchFile on previousStat
06f2eae 
Explains the expected behavior of previousStat in fs.watchFile() when
a watched file disappears and reappears.

Fixes: nodejs#15364
@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. fs Issues and PRs related to the fs subsystem / file system. labels Oct 9, 2017
@jasnell jasnell requested a review from bnoordhuis October 13, 2017 20:19
gireeshpunathil
gireeshpunathil reviewed Oct 14, 2017
View reviewed changes
doc/api/fs.md Outdated
In such cases the reappearance callback will not report the `previousStat` of
the file with all zeroes. This is the correct and expected behavior for
`uv_fs_poll_start()`, the libuv function that `fs.watchFile()` uses internally.

Copy link
Member

Choose a reason for hiding this comment

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

Why this is documented under Appendfile, as opposed to watchFile?

Copy link
Contributor Author

@nikniv nikniv Oct 14, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

@gireeshpunathil Corrected. Thanks for pointing that out! It would be great if you could please review again.

Copy link
Member

Choose a reason for hiding this comment

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

great, thanks!

@nikniv
doc: corrected position of note on fs.watchFile
01a8da4 
Note was previously mistakenly added under fs.appendFile() section.
Now moved to correct position, under fs.watchFile().

The note explains the expected behavior of previousStat in
fs.watchFile() when a watched file disappears and reappears.

Fixes: nodejs#15364
@gireeshpunathil
Copy link
Member

CI: https://ci.nodejs.org/job/node-test-pull-request/10709/

bnoordhuis
bnoordhuis approved these changes Oct 14, 2017
View reviewed changes
Copy link
Member

@bnoordhuis bnoordhuis left a comment

Choose a reason for hiding this comment

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

LGTM with a suggestion.

doc/api/fs.md Outdated

In such cases the reappearance callback will not report the `previousStat` of
the file with all zeroes. This is the correct and expected behavior for
`uv_fs_poll_start()`, the libuv function that `fs.watchFile()` uses internally.
Copy link
Member

Choose a reason for hiding this comment

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

This paragraph can be removed. The first sentence is redundant and the last sentence won't be relevant to most readers, they won't know or care it's implemented by libuv.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bnoordhuis Done!

@nikniv
doc: updated content of note on fs.watchFile
1041e87 
Removed last paragraph, as per suggestions.

The note explains the expected behavior of previousStat in
fs.watchFile() when a watched file disappears and reappears.

Fixes: nodejs#15364
bnoordhuis
bnoordhuis approved these changes Oct 14, 2017
View reviewed changes
Copy link
Member

@bnoordhuis bnoordhuis left a comment

Choose a reason for hiding this comment

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

LGTM, thanks.

@joyeecheung
Copy link
Member

Landed in 371d1db, thanks!

joyeecheung pushed a commit that referenced this pull request Oct 14, 2017
@nikniv @joyeecheung
doc: added note to fs.watchFile on previousStat
371d1db 
Explains the expected behavior of previousStat in fs.watchFile() when
a watched file disappears and reappears.

PR-URL: #16099
Fixes: #15364
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Claudio Rodriguez <cjrodr@yahoo.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
addaleax pushed a commit to ayojs/ayo that referenced this pull request Oct 15, 2017
@nikniv @addaleax
doc: added note to fs.watchFile on previousStat
2774381 
Explains the expected behavior of previousStat in fs.watchFile() when
a watched file disappears and reappears.

PR-URL: nodejs/node#16099
Fixes: nodejs/node#15364
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Claudio Rodriguez <cjrodr@yahoo.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
targos pushed a commit that referenced this pull request Oct 18, 2017
@nikniv @targos
doc: added note to fs.watchFile on previousStat
57b7f40 
Explains the expected behavior of previousStat in fs.watchFile() when
a watched file disappears and reappears.

PR-URL: #16099
Fixes: #15364
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Claudio Rodriguez <cjrodr@yahoo.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
MylesBorins pushed a commit that referenced this pull request Nov 16, 2017
@nikniv @MylesBorins
doc: added note to fs.watchFile on previousStat
b2a7adb 
Explains the expected behavior of previousStat in fs.watchFile() when
a watched file disappears and reappears.

PR-URL: #16099
Fixes: #15364
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Claudio Rodriguez <cjrodr@yahoo.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@MylesBorins MylesBorins mentioned this pull request Nov 21, 2017
Merged
MylesBorins pushed a commit that referenced this pull request Nov 21, 2017
@nikniv @MylesBorins
doc: added note to fs.watchFile on previousStat
116f4a9 
Explains the expected behavior of previousStat in fs.watchFile() when
a watched file disappears and reappears.

PR-URL: #16099
Fixes: #15364
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Claudio Rodriguez <cjrodr@yahoo.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
MylesBorins pushed a commit that referenced this pull request Nov 28, 2017
@nikniv @MylesBorins
doc: added note to fs.watchFile on previousStat
8c070f9 
Explains the expected behavior of previousStat in fs.watchFile() when
a watched file disappears and reappears.

PR-URL: #16099
Fixes: #15364
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Claudio Rodriguez <cjrodr@yahoo.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gireeshpunathil gireeshpunathil gireeshpunathil approved these changes

@claudiorodriguez claudiorodriguez claudiorodriguez approved these changes

@jasnell jasnell jasnell approved these changes

@bnoordhuis bnoordhuis bnoordhuis approved these changes

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.
8 participants
@nikniv @gireeshpunathil @joyeecheung @bnoordhuis @jasnell @claudiorodriguez @MylesBorins @nodejs-github-bot