src: handle bad callback in asyc_wrap#31946
src: handle bad callback in asyc_wrap#31946HarshithaKP wants to merge 2 commits intonodejs:masterfrom
Conversation
src/async_wrap-inl.h
Outdated
There was a problem hiding this comment.
We don’t add using statements to headers (we should probably have a linter rule against that, at least for the v8 symbols…)
There was a problem hiding this comment.
@addaleax, grep "using v8::" src/*.h
I am seeing this pattern in few files already. If I remove using phrase, is there any way I can elevate ?
There was a problem hiding this comment.
Yes, these other files should also not use it.
There are no shorthands in that case, we do spell out the entire type in headers.
There was a problem hiding this comment.
@addaleax,thanks. Reverted the changes. What is the rationale for this ?
There was a problem hiding this comment.
I mean, why we want to skip for headers ? Is there any performance reason or something else ?
There was a problem hiding this comment.
@HarshithaKP Because then the symbols are available in any source file that includes this header, even if that’s not obvious. For example, this code would fail to compile if example.h has a using v8::Array; line:
#include "example.h"
class Array {};Align with the MaybeLocal<> API contract
2a91f94 to
fc53c8d
Compare
Align with the MaybeLocal<> API contract PR-URL: #31946 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>
|
landed in 757e203 |
Align with the MaybeLocal<> API contract PR-URL: #31946 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>
Align with the MaybeLocal<> API contract PR-URL: #31946 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>
Align with the MaybeLocal<> API contract PR-URL: #31946 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>
Align with the MaybeLocal<> API contract PR-URL: #31946 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>
Align with the MaybeLocal<> API contract PR-URL: #31946 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>
src: handle bad callback in asyc_wrap
Align with the MaybeLocal<> API contract
Refs:
node/deps/v8/include/v8.h
Lines 345 to 349 in 9403250
addresses an @addaleax 's TODO
src: elevate v8 namespaces
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes