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.

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

fs: undocument fs.open's 'rs' mode #6732

Merged
merged 1 commit into from
May 13, 2016
Merged

fs: undocument fs.open's 'rs' mode #6732

merged 1 commit into from
May 13, 2016

Conversation

saghul
Copy link
Member

@saghul saghul commented May 13, 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)

fs

Description of change

See: #6730

R= @bnoordhuis

@bnoordhuis
Copy link
Member

Can you leave lib/fs.js untouched? Just in case O_RDONLY | O_SYNC is meaningful on some platform.

@bnoordhuis
Copy link
Member

Documentation commit LGTM BTW.

@saghul
Copy link
Member Author

saghul commented May 13, 2016

Sure, will do in a bit.
On May 13, 2016 10:37, "Ben Noordhuis" notifications@github.com wrote:

Can you leave lib/fs.js untouched? Just in case O_RDONLY | O_SYNC is
meaningful on some platform.


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#6732 (comment)

Using O_SYNC with O_RDONLY is basically a noop.

Closes: nodejs#6730
PR-URL: nodejs#6732
Reviewed-By: Ben Noorhduis <info@bnoordhuis.nl>
@saghul saghul merged commit a661e26 into nodejs:master May 13, 2016
evanlucas pushed a commit that referenced this pull request May 17, 2016
Using O_SYNC with O_RDONLY is basically a noop.

Closes: #6730
PR-URL: #6732
Reviewed-By: Ben Noorhduis <info@bnoordhuis.nl>
@ChALkeR ChALkeR added fs Issues and PRs related to the fs subsystem / file system. doc Issues and PRs related to the documentations. labels May 28, 2016
@MylesBorins
Copy link
Contributor

setting this as don't land for now... please feel free to update if you feel it should be backported @saghul

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. fs Issues and PRs related to the fs subsystem / file system.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants