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: fix .write() not coercing non-string values #1102

Closed

Conversation

Fishrock123
Copy link
Contributor

@Fishrock123 Fishrock123 added the fs Issues and PRs related to the fs subsystem / file system. label Mar 8, 2015
@Fishrock123
Copy link
Contributor Author

test is just adapted from test-fs-write.js

Fishrock123 added a commit that referenced this pull request Mar 8, 2015
Fixes: #1098
PR-URL: #1102
Reviewed-By: Brendan Ashworth <brendan.ashworth@me.com>
@brendanashworth
Copy link
Contributor

LGTM. Thanks @Fishrock123, merged in cf565b5.

@rvagg
Copy link
Member

rvagg commented Mar 8, 2015

@brendanashworth since @Fishrock123 is a collaborator it's probably best to let him merge it. I personally would have preferred a bit more time to have some more eyes on this before merging as it's not as trivial as a doc fix.

@brendanashworth
Copy link
Contributor

@rvagg sorry, you're right, I probably should've let it sizzle - then I buried it with another commit. Won't happen again :)

@Fishrock123
Copy link
Contributor Author

The behavior appears to have been incorrect from the start: 7ca77ea#diff-9a205ef7ee967ee32efee02e58b3482dR484

No tests were added when the API was added, and thus it was not caught.

@Fishrock123 Fishrock123 deleted the fix-fs-write-string-coerce branch March 26, 2015 19:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fs Issues and PRs related to the fs subsystem / file system.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants