Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34.3k
src: elevate v8 namespaces referenced#24657
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
src/async_wrap.cc Outdated
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.
I think you can now unwrap this into a single line.
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.
Do you know what is the max width of line? Linter could break this, no?
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.
The max length is 80 and if you exceed that the linter will pick it up. You can run make lint-cpp to check any changes.
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.
Rebased and wrapped!
danbev commented Nov 28, 2018
juanarbol commented Nov 29, 2018
@rvagg 🤦♂️ My bad, at this hour I still thinking why I rebase, what a mess... |
7d404f1 to 4996605CompareTrott commented Nov 29, 2018
@juanarbol I checked out your branch and ran |
Trott commented Nov 29, 2018
juanarbol commented Nov 29, 2018
@Trott I think it's ok! Thank you!!! |
Trott commented Nov 30, 2018
Resume Build CI: https://ci.nodejs.org/job/node-test-pull-request/19077/ |
Trott commented Nov 30, 2018
Optimistic Resume Build CI: https://ci.nodejs.org/job/node-test-pull-request/19091/ |
Trott commented Dec 1, 2018
Resume Build CI: https://ci.nodejs.org/job/node-test-pull-request/19105/ |
Trott commented Dec 1, 2018
Landed in d4273c4 |
PR-URL: nodejs#24657 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Minwoo Jung <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]>
PR-URL: #24657 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Minwoo Jung <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]>
PR-URL: nodejs#24657 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Minwoo Jung <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]>
PR-URL: #24657 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Minwoo Jung <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]>
PR-URL: #24657 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Minwoo Jung <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]>
I saw non elevated v8 namespaces, and
v8::HandleScopealready elevated but invocated asv8::HandleScopenot justHandleScopeChecklist
make -j4 test(UNIX), orvcbuild test(Windows) passes