Test & document numeric flags to fs.open on unix#3641

Closed

XeCycle wants to merge 3 commits intonodejs:masterfrom

XeCycle:open-num-flag

Contributor

XeCycle commented Nov 3, 2015

For #3628.

Do we prefer two commits here or one? Also, shall we backport the doc commit to older versions?

mscdex added the fs label

thefourtheye reviewed

View reviewed changes

test/parallel/test-fs-open-numeric-flags-posix.js Outdated

Contributor

thefourtheye Nov 3, 2015

Use assert.throws instead

XeCycle force-pushed the open-num-flag branch from 12b5803 to 053513f Compare

November 3, 2015 05:46

bnoordhuis reviewed

View reviewed changes

doc/api/fs.markdown Outdated

Member

bnoordhuis Nov 3, 2015

Not just POSIX, it works on Windows too. Not all flags from require('constants') are available but the common ones are.

XeCycle force-pushed the open-num-flag branch from 053513f to 9306682 Compare

November 3, 2015 11:34

jasnell added the lts-watch-v4.x label

sindresorhus reviewed

View reviewed changes

doc/api/fs.markdown

sindresorhus Nov 4, 2015

Not all users know what open(2) means. Would be nice if this was linkified to some docs on it or even better the numbers being documented here too.

Contributor Author

XeCycle Nov 4, 2015

Well, there are many (2) notations in this file; do you want to linkify every one? I suppose that should be another PR anyway.

sindresorhus Nov 4, 2015

Yes, but I can do it in another PR.

sindresorhus mentioned this pull request

Document which open(2) numbers are supported on Windows libuv/libuv#606

Closed

XeCycle force-pushed the open-num-flag branch from 9306682 to 2abbf90 Compare

November 9, 2015 12:53

bnoordhuis reviewed

View reviewed changes

doc/api/fs.markdown Outdated

Member

bnoordhuis Nov 10, 2015

I'd write this as "should be opened in blocking mode".

Also, s/could/should/

Contributor Author

XeCycle Nov 11, 2015

fcntl can set blocking mode after open, so imo "opened in blocking mode" may be inaccurate.

Okay will s/could/should/, not being a native speaker I have to trust you :)

Member

bnoordhuis commented Nov 10, 2015

Left some comments.

@XeCycle GH doesn't send notifications for new or changed commits, better leave a comment or it won't get noticed until someone goes through the list of open pull requests (like I'm currently doing.)

XeCycle force-pushed the open-num-flag branch from 2abbf90 to 4284cd1 Compare

November 11, 2015 00:44

Contributor Author

XeCycle commented Nov 11, 2015

@bnoordhuis sorry am not aware of that, commenting this time.

Rebased to latest master, and did s/could/should/.

Member

bnoordhuis commented Nov 11, 2015

LGTM with a question. CI: https://ci.nodejs.org/job/node-test-pull-request/701/

Member

jasnell commented Nov 13, 2015

@XeCycle ... this LGTM but can I ask you to please rebase and update?

Contributor Author

XeCycle commented Nov 14, 2015

@jasnell will do that in 48hr. Now out on a vacation.

XeCycle force-pushed the open-num-flag branch 2 times, most recently from 4f6cb6f to 4c90d94 Compare

November 16, 2015 01:18

Contributor Author

XeCycle commented Nov 16, 2015

@bnoordhuis @jasnell rebased, and reworded translation on windows to say "where applicable". Please check if these still LGTY.

Member

jasnell commented Nov 16, 2015

LGTM (with a comment)

jasnell reviewed

View reviewed changes

doc/api/fs.markdown Outdated

Member

jasnell Nov 16, 2015

Unfortunately, this last sentence is a rather unclear. I'd recommend including a bit more detail about what is translated to what...

Contributor Author

XeCycle Nov 16, 2015

Ah seems a typo there, I wanted to say "where applicable". Yes, it is unclear; but details imo should go to libuv docs, but no one is taking on that. What about adding a few examples, "e.g. O_EXCL to xxx, O_RDWR to xxx..."?

Member

jasnell Nov 16, 2015

Examples would be great, I think.

XeCycle force-pushed the open-num-flag branch from 4c90d94 to 62d3163 Compare

November 17, 2015 00:48

Contributor Author

XeCycle commented Nov 17, 2015

Added a few examples to flags translation.

bnoordhuis closed this

XeCycle deleted the open-num-flag branch

November 23, 2015 01:39

Fishrock123 mentioned this pull request

deps: upgrade npm to 3.5.0 #4032

Closed

MylesBorins pushed a commit that referenced this pull request


          doc: clarify that fs streams expect blocking fd

41453b3

This clarifies that fs.createReadStream and fs.createWriteStream, when
passed a fd, expects the fd to be blocking, and suggests net.Socket as
an alternative.

PR-URL: #3641
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>

MylesBorins pushed a commit that referenced this pull request


          test: numeric flags to fs.open

0482ec4

This has been supperted for long but never tested nor documented.

PR-URL: #3641
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>

MylesBorins pushed a commit that referenced this pull request


          doc: numeric flags to fs.open

d2d4f17

This has been supperted for long but never tested nor documented.

PR-URL: #3641
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>

MylesBorins added lts-landed-on-v4.x and removed lts-watch-v4.x labels

rvagg pushed a commit that referenced this pull request


          doc: clarify that fs streams expect blocking fd

b96f2a8

This clarifies that fs.createReadStream and fs.createWriteStream, when
passed a fd, expects the fd to be blocking, and suggests net.Socket as
an alternative.

PR-URL: #3641
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>

rvagg pushed a commit that referenced this pull request


          test: numeric flags to fs.open

660b688

This has been supperted for long but never tested nor documented.

PR-URL: #3641
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>

rvagg pushed a commit that referenced this pull request


          doc: numeric flags to fs.open

511c1e7

This has been supperted for long but never tested nor documented.

PR-URL: #3641
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>

rvagg pushed a commit that referenced this pull request


          doc: clarify that fs streams expect blocking fd

f4ca007

This clarifies that fs.createReadStream and fs.createWriteStream, when
passed a fd, expects the fd to be blocking, and suggests net.Socket as
an alternative.

PR-URL: #3641
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>

rvagg pushed a commit that referenced this pull request


          test: numeric flags to fs.open

f9fe0ae

This has been supperted for long but never tested nor documented.

PR-URL: #3641
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>

rvagg pushed a commit that referenced this pull request


          doc: numeric flags to fs.open

77251d9

This has been supperted for long but never tested nor documented.

PR-URL: #3641
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>

This was referenced Dec 8, 2015

Release proposal: v5.2.0 #4180

Closed

Release proposal: v5.2.0 (Stable) #4181

Merged

MylesBorins added land-on-v4.x and removed lts-landed-on-v4.x labels

jasnell mentioned this pull request

Release 4.2.4 Planning #4321

Closed

jasnell pushed a commit that referenced this pull request


          doc: clarify that fs streams expect blocking fd

This clarifies that fs.createReadStream and fs.createWriteStream, when
passed a fd, expects the fd to be blocking, and suggests net.Socket as
an alternative.

PR-URL: #3641
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>

jasnell pushed a commit that referenced this pull request


          test: numeric flags to fs.open

9a6c9fa

This has been supperted for long but never tested nor documented.

PR-URL: #3641
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>

jasnell pushed a commit that referenced this pull request


          doc: numeric flags to fs.open

4c382e7

This has been supperted for long but never tested nor documented.

PR-URL: #3641
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>

jasnell pushed a commit that referenced this pull request


          doc: clarify that fs streams expect blocking fd

bae03f2

This clarifies that fs.createReadStream and fs.createWriteStream, when
passed a fd, expects the fd to be blocking, and suggests net.Socket as
an alternative.

PR-URL: #3641
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>

jasnell pushed a commit that referenced this pull request


          test: numeric flags to fs.open

1570d32

This has been supperted for long but never tested nor documented.

PR-URL: #3641
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>

jasnell pushed a commit that referenced this pull request


          doc: numeric flags to fs.open

b10433b

This has been supperted for long but never tested nor documented.

PR-URL: #3641
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc fs test