-
Notifications
You must be signed in to change notification settings - Fork 30k
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 async-wrap -> async_wrap #17022
Conversation
@danbev is there a good motivation (even something like a simpler glob in some script)? |
@refack I don't see why not, the current naming is pretty likely to lead to typos during development. (Speaking from personal experience with ReqWrap...) (I don't think this affects backporting or anything, does it?) |
The motivation is simply that I've noticed this inconsistency and at one point thought it might mean something that some source files were named differently. As there does not seem to be any reason for this, having them named consistently seems to make sense to me. |
Any preferences to how these commits should land, squashed as one single commit (and update the commit message to reflect that) or as separate commits? The only reason for multiple commit would be to make reverting easier if required. |
42fb834
to
3445dbb
Compare
test/linux-openssl110 failures look unrelatednot ok 1867 parallel/test-http2-create-client-connect # TODO : Fix flaky test
---
duration_ms: 120.35
severity: fail
stack: |-
timeout
...
not ok 101 async-hooks/test-signalwrap
---
duration_ms: 0.415
severity: fail
stack: |-
Mismatched onsigusr2 function calls. Expected exactly 2, actual 1.
at Object.exports.mustCall (/home/iojs/build/workspace/node-test-commit-linux-linked/nodes/ubuntu1604_sharedlibs_openssl110_x64/test/common/index.js:490:10)
at Object.<anonymous> (/home/iojs/build/workspace/node-test-commit-linux-linked/nodes/ubuntu1604_sharedlibs_openssl110_x64/test/async-hooks/test-signalwrap.js:14:30)
at Module._compile (module.js:644:30)
at Object.Module._extensions..js (module.js:655:10)
at Module.load (module.js:563:32)
at tryModuleLoad (module.js:506:12)
at Function.Module._load (module.js:498:3)
at Function.Module.runMain (module.js:685:10)
at startup (bootstrap_node.js:192:16)
... |
This commit renames async-wrap to async_wrap for consitency with other c++ source files.
This commit renames base-object to base_object for consitency with other c++ source files.
This commit renames req-wrap to req_wrap consitency with other c++ source files.
3445dbb
to
e2249b8
Compare
Rebased CI: https://ci.nodejs.org/job/node-test-pull-request/11494/ |
test/linux-debug failures look unrelatednot ok 499 parallel/test-error-reporting # TODO : Fix flaky test
---
duration_ms: 70.88
severity: flaky
stack: |-
assert.js:42
throw new errors.AssertionError({
^
AssertionError [ERR_ASSERTION]: false == true
at /home/iojs/build/workspace/node-test-commit-linux-linked/nodes/ubuntu1604_sharedlibs_debug_x64/test/parallel/test-error-reporting.js:80:10
at /home/iojs/build/workspace/node-test-commit-linux-linked/nodes/ubuntu1604_sharedlibs_debug_x64/test/common/index.js:522:15
at /home/iojs/build/workspace/node-test-commit-linux-linked/nodes/ubuntu1604_sharedlibs_debug_x64/test/parallel/test-error-reporting.js:38:5
at ChildProcess.exithandler (child_process.js:279:5)
at ChildProcess.emit (events.js:159:13)
at maybeClose (internal/child_process.js:943:16)
at Process.ChildProcess._handle.onexit (internal/child_process.js:220:5)
... not ok 789 parallel/test-http-pipeline-flood
---
duration_ms: 58.41
severity: fail
stack: |-
Mismatched <anonymous> function calls. Expected exactly 1, actual 2.
at Object.exports.mustCall (/home/iojs/build/workspace/node-test-commit-linux-linked/nodes/ubuntu1604_sharedlibs_debug_x64/test/common/index.js:490:10)
at Server.<anonymous> (/home/iojs/build/workspace/node-test-commit-linux-linked/nodes/ubuntu1604_sharedlibs_debug_x64/test/parallel/test-http-pipeline-flood.js:59:35)
at Object.onceWrapper (events.js:254:19)
at Server.emit (events.js:159:13)
at emitListeningNT (net.js:1394:10)
at _combinedTickCallback (internal/process/next_tick.js:135:11)
at process._tickCallback (internal/process/next_tick.js:180:9)
at Function.Module.runMain (module.js:687:11)
at startup (bootstrap_node.js:192:16) not ok 1856 parallel/test-zlib
---
duration_ms: 480.251
severity: fail
stack: |-
timeout
... not ok 1898 sequential/test-inspector-contexts # TODO : Fix flaky test
---
duration_ms: 6.830
severity: crashed
stack: |-
oh no!
exit code: CRASHED (Signal: 4)
... not ok 1934 sequential/test-repl-timeout-throw
---
duration_ms: 10.517
severity: fail
stack: |-
> ''
> THROW 0
ReferenceError: XXX is not defined
at Timeout.thrower [as _onTimeout] (repl:1:116)
at ontimeout (timers.js:478:11)
at tryOnTimeout (timers.js:302:5)
at Timer.listOnTimeout (timers.js:262:5)
> undefined
> ... ... ... ..... ..... ....... ....... ..... ... ''
> THROW 1
ReferenceError: XXX is not defined
at EventEmitter.thrower (repl:1:116)
at EventEmitter.emit (events.js:159:13)
> 2
var throws = 0;process.on("exit",function(){console.log(throws)});function thrower(){console.log("THROW",throws++);XXX};setTimeout(thrower);""
fs.readFile("/home/iojs/build/workspace/node-test-commit-linux-linked/nodes/ubuntu1604_sharedlibs_debug_x64/test/sequential/test-repl-timeout-throw.js", thrower);
setTimeout(function() {
const events = require("events");
var e = new events.EventEmitter;
process.nextTick(function() {
e.on("x", thrower);
setTimeout(function() {
e.emit("x");
});
});
});"";
assert.js:42
throw new errors.AssertionError({
^
AssertionError [ERR_ASSERTION]: '> 2' === '> 3'
at ChildProcess.<anonymous> (/home/iojs/build/workspace/node-test-commit-linux-linked/nodes/ubuntu1604_sharedlibs_debug_x64/test/sequential/test-repl-timeout-throw.js:58:10)
at ChildProcess.emit (events.js:159:13)
at maybeClose (internal/child_process.js:943:16)
at Process.ChildProcess._handle.onexit (internal/child_process.js:220:5)
... |
This commit renames async-wrap to async_wrap for consitency with other c++ source files. PR-URL: #17022 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
This commit renames base-object to base_object for consitency with other c++ source files. PR-URL: #17022 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
This commit renames req-wrap to req_wrap consitency with other c++ source files. PR-URL: #17022 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
This commit renames async-wrap to async_wrap for consitency with other c++ source files. PR-URL: #17022 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
This commit renames base-object to base_object for consitency with other c++ source files. PR-URL: #17022 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
This commit renames req-wrap to req_wrap consitency with other c++ source files. PR-URL: #17022 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
@danbev this conflicts on v8.x, would you mind raising a backport (assuming it's just a find-replace). |
This commit renames async-wrap to async_wrap for consitency with other c++ source files. PR-URL: nodejs#17022 Backport-PR-URL: nodejs#18179 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
This commit renames async-wrap to async_wrap for consitency with other
c++ source files.
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passesAffected core subsystem(s)
src