-
Notifications
You must be signed in to change notification settings - Fork 29.1k
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: rename fs_req_wrap -> FSReqWrapSync #19614
Conversation
This commit renames fs_req_wrap to FSReqWrapSync to make it consistent with most of the other classes in the code base.
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.
Thanks, I was thinking about renaming this but struggled with coming up with a good name...FSReqWrapSync
seems perfect
@@ -775,7 +775,7 @@ void Access(const FunctionCallbackInfo<Value>& args) { | |||
uv_fs_access, *path, mode); | |||
} else { // access(path, mode, undefined, ctx) | |||
CHECK_EQ(argc, 4); | |||
fs_req_wrap req_wrap; | |||
FSReqWrapSync req_wrap; |
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.
In retrospect variables like this can be renamed req_wrap_sync
and those FSReqBase*
can be renamed req_wrap_async
to avoid the confusion of name shadowing..not really what this PR needs to deal with though.
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 noticed that too and think we should change that. I'll follow up this is in a different PR. Thanks!
node-test-commit failure looks unrelatedBuilding addon /home/iojs/build/workspace/node-test-commit-plinux/nodes/ppcle-ubuntu1404/test/addons/stringbytes-external-exceed-max/
gyp '/home/iojs/build/workspace/node-test-commit-plinux/nodes/ppcle-ubuntu1404/test/addons-napi/test_symbol/build/config.gypi',
make[2]: write error
gyp info spawn argsmake[1]: *** [doc-only] Error 1
'-I',
gypmake[1]: *** Waiting for unfinished jobs.... |
Landed in 376f949. |
This commit renames fs_req_wrap to FSReqWrapSync to make it consistent with most of the other classes in the code base. PR-URL: #19614 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>
This commit renames the req_wrap variable to use an -async/-sync suffix to avoid cases where the variables were being shadowed. Refs: nodejs#19614
This commit renames the req_wrap variable to use an -async/-sync suffix to avoid cases where the variables were being shadowed. PR-URL: #19628 Refs: #19614 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: James M Snell <[email protected]>
src: rename fs_req_wrap -> FSReqWrapSync nodejs/node#19614
This commit renames fs_req_wrap to FSReqWrapSync to make it consistent
with most of the other classes in the code base.
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes