-
Notifications
You must be signed in to change notification settings - Fork 604
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
connection: use ~~process.nextTick~~ setImmediate #166
Conversation
@@ -260,7 +264,7 @@ Connection.prototype.createAuthorizedReq = function(reqOpts, callback) { | |||
} | |||
|
|||
if (this.isConnected()) { | |||
onConnected(); | |||
process.nextTick(onConnected); |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
We should be using Pasted here for easy reading:
|
From the link:
We're just executing a callback. I'm inclined to stick to nextTick for convention reasons, as well as it's the model scenario for async-ifying sync methods according to the docs: process.nextTick. |
We should only use |
My only reasons for nextTick over setImmediate are based purely on the convention and nextTick documentation. I haven't seen a use of setImmediate to async a sync method, and would feel more comfortable if I did. I do see your points, however I don't think they affect us. It would take a very serious misuse of our library to stack a But, yolo. I'll switch it to setImmediate, 'cause why not. It doesn't seem to hurt anything, other than offer a minimal performance loss. Feels weird, but what doesn't in Node? (@rakyll - feel free to chime in if you prefer it to stick to nextTick) |
Yeah before either of us make a decision, I would like to hear what @rakyll |
With node v11.0, immediate queue will be able to tick between process ticks. I think that is going to solve the performance concern mentioned above. Otherwise, LGTM. See this test: https://github.com/joyent/node/blob/857975d5e7e0d7bf38577db0478d9e5ede79922e/test/simple/test-timers-immediate-queue.js |
@stephenplusplus You can probably merge this whenever. |
Woo! Thanks for saving us, Ryan. setImmediate for the win! |
connection: use setImmediate to simulate async.
😄 👍 |
* build: add Kokoro configs for autorelease * build: add Kokoro configs for autorelease * chore: remove CircleCI config
* chore: Configure Dialogflow CX for Ruby clients PiperOrigin-RevId: 392056283 Source-Link: googleapis/googleapis@beb5d2b Source-Link: googleapis/googleapis-gen@e93b804 * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
- [ ] Regenerate this pull request now. PiperOrigin-RevId: 470911839 Source-Link: googleapis/googleapis@3527566 Source-Link: googleapis/googleapis-gen@f16a1d2 Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiZjE2YTFkMjI0ZjAwYTYzMGVhNDNkNmE5YTFhMzFmNTY2ZjQ1Y2RlYSJ9 feat: accept google-gax instance as a parameter Please see the documentation of the client constructor for details. PiperOrigin-RevId: 470332808 Source-Link: googleapis/googleapis@d4a2367 Source-Link: googleapis/googleapis-gen@e97a1ac Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiZTk3YTFhYzIwNGVhZDRmZTczNDFmOTFlNzJkYjdjNmFjNjAxNjM0MSJ9
[](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [jsdoc](https://togithub.com/jsdoc/jsdoc) | [`^3.6.6` -> `^4.0.0`](https://renovatebot.com/diffs/npm/jsdoc/3.6.11/4.0.0) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>jsdoc/jsdoc</summary> ### [`v4.0.0`](https://togithub.com/jsdoc/jsdoc/blob/HEAD/CHANGES.md#​400-November-2022) [Compare Source](https://togithub.com/jsdoc/jsdoc/compare/3.6.11...084218523a7d69fec14a852ce680f374f526af28) - JSDoc releases now use [semantic versioning](https://semver.org/). If JSDoc makes backwards-incompatible changes in the future, the major version will be incremented. - JSDoc no longer uses the [`taffydb`](https://taffydb.com/) package. If your JSDoc template or plugin uses the `taffydb` package, see the [instructions for replacing `taffydb` with `@jsdoc/salty`](https://togithub.com/jsdoc/jsdoc/tree/main/packages/jsdoc-salty#use-salty-in-a-jsdoc-template). - JSDoc now supports Node.js 12.0.0 and later. </details> --- ### Configuration 📅 **Schedule**: Branch creation - "after 9am and before 3pm" (UTC), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/nodejs-appengine-admin). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC4xNy4xIiwidXBkYXRlZEluVmVyIjoiMzQuMTcuMSJ9-->
samples: pull in latest typeless bot, clean up some comments Source-Link: https://togithub.com/googleapis/synthtool/commit/0a68e568b6911b60bb6fd452eba4848b176031d8 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-nodejs:latest@sha256:5b05f26103855c3a15433141389c478d1d3fe088fb5d4e3217c4793f6b3f245e
🤖 I have created a release *beep* *boop* --- ## [2.0.3](https://togithub.com/googleapis/nodejs-service-control/compare/v2.0.2...v2.0.3) (2022-11-10) ### Bug Fixes * **deps:** Use google-gax v3.5.2 ([#163](https://togithub.com/googleapis/nodejs-service-control/issues/163)) ([f883825](https://togithub.com/googleapis/nodejs-service-control/commit/f88382517737c02ea2f0f9fa4e6c624c7a67c6b8)) * Preserve default values in x-goog-request-params header ([#156](https://togithub.com/googleapis/nodejs-service-control/issues/156)) ([0548559](https://togithub.com/googleapis/nodejs-service-control/commit/0548559a942b3f9830d49f9fa54aa75f259d355d)) * Regenerated protos JS and TS definitions ([#166](https://togithub.com/googleapis/nodejs-service-control/issues/166)) ([f9348ff](https://togithub.com/googleapis/nodejs-service-control/commit/f9348ff6913fe16f8dcf30dc81c96748d445c328)) --- This PR was generated with [Release Please](https://togithub.com/googleapis/release-please). See [documentation](https://togithub.com/googleapis/release-please#release-please).
- [ ] Regenerate this pull request now. PiperOrigin-RevId: 439356405 Source-Link: googleapis/googleapis@afa2ba1 Source-Link: googleapis/googleapis-gen@3e40c17 Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiM2U0MGMxN2UxNTEwYzk1ZmFiNThmYzIxNDNjY2I2MWNjZWNhNTk4OSJ9
Committer: @miraleung PiperOrigin-RevId: 310415142 Source-Author: Google APIs <[email protected]> Source-Date: Thu May 7 12:34:02 2020 -0700 Source-Repo: googleapis/googleapis Source-Sha: 684dfea7decfeca7a7526ea96a8e9256694dd5d8 Source-Link: googleapis/googleapis@684dfea
This is really just two types of changes in many places:
Use...
...as opposed to anything else.
Use
process.nextTick
in two places to simulate an async operation.