Skip to content
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

create a detach helper and use detach helper in replace of nextTick #1168

Merged
merged 2 commits into from
Oct 15, 2014
Merged

create a detach helper and use detach helper in replace of nextTick #1168

merged 2 commits into from
Oct 15, 2014

Conversation

seanstrom
Copy link
Contributor

In response to #748

Created a new helper to pick setImmediate when available and process.nextTick when not.
And I then replaced code that relied directly on process.nextTick

Review @mikeal @emkay @nylen @FredKSchott

@FredKSchott
Copy link
Contributor

+1 but can we call it something else? Maybe callAsync()? detach() sounds like you're detaching the function from somethings. (apologies in advance for the bike shed)

@nylen
Copy link
Member

nylen commented Oct 15, 2014

I like callAsync better too. I'm not crazy about either of those names, but I can't think of a better one, so 👍

@FredKSchott
Copy link
Contributor

You could go all Java with setImmedieteOrNextTick()

@nylen
Copy link
Member

nylen commented Oct 15, 2014

👎 needs more factories and IAbstractSetImmediateOrNextTickImpl 💩

@emkay
Copy link
Member

emkay commented Oct 15, 2014

haha. I've never done any java, but I actually like setImmediteteOrNextTick() the best because it clearly states what it is for. callAsync and detach are somewhat ambiguous, but hey it's just a name.

@seanstrom
Copy link
Contributor Author

👍 on @emkay's thoughts

@nylen
Copy link
Member

nylen commented Oct 15, 2014

setImmediateOrNextTick is fine with me.

@seanstrom
Copy link
Contributor Author

What about defer as the name.
I spent some time researching what process.nextTick and setImmediate do and came up with that name.
But maybe its still not appropriate.

@nylen
Copy link
Member

nylen commented Oct 15, 2014

👍 for defer, I like that best so far.

@FredKSchott
Copy link
Contributor

Woo! 🚲 🏡 complete :)

FredKSchott added a commit that referenced this pull request Oct 15, 2014
…-available

create a detach helper and use detach helper in replace of nextTick
@FredKSchott FredKSchott merged commit 105e74b into request:master Oct 15, 2014
@seanstrom seanstrom deleted the support/use-setimmediate-when-available branch October 15, 2014 21:57
@ryanseys
Copy link

Sweeet! :) Thanks for this!

nylen pushed a commit to nylen/request that referenced this pull request Oct 17, 2014
…te-when-available

create a detach helper and use detach helper in replace of nextTick
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants