Uh oh!
There was an error while loading. Please reload this page.
fs: Fix default params for fs.write(Sync) (backport to v6) - #13179
fs: Fix default params for fs.write(Sync) (backport to v6)#13179papandreou wants to merge 1 commit into
Conversation
papandreou
commented
May 23, 2017
There's something up with my C++ compiler, so I wasn't able to test it. However, the cherry-pick applied cleanly except for some minor conflicts in the tests, so I think there's a good chance it'll pass on CI in this state. |
MylesBorins
commented
May 30, 2017
should this be semver-minor? |
gibfahn
commented
May 30, 2017
There are three comments in #7856 that say that it should be |
6ef8c5b to
4c2fa3dComparef9419c2 to
403c465Compareaaf4e13 to
31f572cCompare(Backported from nodejs#7856 to v6.x-staging) Add support for fs.write(fd, buffer, cb) and fs.write(fd, buffer, offset, cb) as documented at https://nodejs.org/api/fs.html#fs_fs_write_fd_data_position_encoding_callback and equivalently for fs.writeSync Update docs and code comments to reflect the implementation.
c542b2c to
93e9752Comparepapandreou
commented
Sep 18, 2017
Rebased and fixed conflicts. |
Fishrock123
commented
Sep 19, 2017
This change affected someone: #10242 Seems like it could cause some interruption if backported, though it would ideally be a good fix. |
papandreou
commented
Sep 19, 2017
I still think it's likely to save more trouble than it's causing, but OTOH it doesn't seem like there's a lot of interest getting this backport landed. I don't care much myself, as all my projects are using at least 7.10.1 in production. |
BridgeAR
commented
Sep 22, 2017
@MylesBorins as you are mainly handling the v6 backports, PTAL |
b811464 to
2c8fe97Compare355249a to
0597d3fCompareMylesBorins
commented
Oct 10, 2017
@nodejs/release has anyone had a chance to review this to confirm if we want to land it or not? As @Fishrock123 mentioned above this has caused regressions in the past |
gibfahn
commented
Oct 10, 2017
Looking at #10242 (comment), it looks like the one user who raised this was trying to work out why the behaviour was wrong in the older version (it worked on 7.2.0 but is broken on 7.1.0). So I'd be +1 on this landing. Before if you didn't provide the length the write would fail, now it succeeds. |
Add support for fs.write(fd, buffer, cb) and fs.write(fd, buffer, offset, cb) as documented at https://nodejs.org/api/fs.html#fs_fs_write_fd_data_position_encoding_callback and equivalently for fs.writeSync Update docs and code comments to reflect the implementation. Backport-PR-URL: #13179 PR-URL: #7856 Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Brian White <mscdex@mscdex.net> Reviewed-By: Yorkie Liu <yorkiefixer@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
MylesBorins
commented
Oct 16, 2017
landed in 968e564 |
Add support for fs.write(fd, buffer, cb) and fs.write(fd, buffer, offset, cb) as documented at https://nodejs.org/api/fs.html#fs_fs_write_fd_data_position_encoding_callback and equivalently for fs.writeSync Update docs and code comments to reflect the implementation. Backport-PR-URL: #13179 PR-URL: #7856 Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Brian White <mscdex@mscdex.net> Reviewed-By: Yorkie Liu <yorkiefixer@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
(Backported from #7856 to v6.x-staging, requested by @sam-github on #7856 (comment))
Add support for fs.write(fd, buffer, cb) and fs.write(fd, buffer, offset, cb)
as documented at
https://nodejs.org/api/fs.html#fs_fs_write_fd_data_position_encoding_callback
and equivalently for fs.writeSync
Update docs and code comments to reflect the implementation.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
fs