-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
src: add ASSERT_EQ style macros #529
Conversation
int64_t len = GET_TRUNCATE_LENGTH(args[1]); | ||
// FIXME(bnoordhuis) It's questionable to reject non-ints here but still | ||
// allow implicit coercion from null or undefined to zero. Probably best | ||
// handled in lib/fs.js. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agreed. it's much easier to handle value coercion from JS. even if that means all there is is a minor wrapper that does a couple coercions and checks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good, I'll address that in a follow-up PR.
LGTM |
Add ASSERT counterparts to the CHECK_EQ/CHECK_NE/etc. family of macros. PR-URL: nodejs#529 Reviewed-By: Trevor Norris <[email protected]>
Rename the misnomers ASSERT_IS_STRING_OR_BUFFER and ASSERT_IS_BUFFER. Said macros don't assert, they throw a TypeError and return. PR-URL: nodejs#529 Reviewed-By: Trevor Norris <[email protected]>
Remove a few unused or barely used macros from src/node_file.cc. PR-URL: nodejs#529 Reviewed-By: Trevor Norris <[email protected]>
d0d47e7
to
668420d
Compare
R=@trevnorris?