-
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
Fix timers #336
Fix timers #336
Conversation
@@ -15,8 +15,6 @@ | |||
var common = require('../common'); | |||
|
|||
var Timer = process.binding('timer_wrap').Timer; | |||
Timer.now = function() { return ++Timer.now.ticks; }; |
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.
Unfortunately, I don't fully understand what @bnoordhuis meant. However, the test runs without it, i.e. check it out correctly this._onTimeout
.
Sorry for letting this pull request fall through the cracks. I think it would be best (in the hope of merging) to split the pull request into separate PRs for each individual commit (and perform a rebase on each, as necessary). Some of these commits will probably bring more discussion than the others and I don't think these can all be merged at once. |
@loyd are you still up to updating this PR? Going to close otherwise. |
@Fishrock123 at least two of the three commits here I would like to see merged into the repository, but I think since they are packaged all together they aren't very welcomed. If you could hold off on closing this so I could resubmit the commits one at a time, that'd be great - I'd be up to update this for him. |
Sorry for the delay in response. |
@loyd thanks for rebasing. I'd like to see I'm not entirely sure what |
All the commits in this pull request need some serious rebasing by now, and since they need to be resubmitted in separate pull requests anyways, I'll close this for now. We'd still welcome new pull requests with these changes, but as they are now (bundled and in need of a rebase), we cannot continue to work on them from this PR. If you disagree, you can always reopen this. Thanks! p.s. I tried rebasing the "fix for unref'd timers", but I got a segfault. 😛 |
Adoption of nodejs/node-v0.x-archive#8120.