-
Notifications
You must be signed in to change notification settings - Fork 41
Better setTimeout
APIs
#86
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
Conversation
…the callback function
6dc3d4c
to
0f990f1
Compare
What is preventing this branch from landing? Just review from @caleb-distributive ? |
It's missing |
To make this pull request land sooner, I decided to split it into separate PRs. |
c6bf140
to
4b5cfab
Compare
|
||
// Schedule job to the running Python event-loop | ||
PyEventLoop loop = PyEventLoop::getRunningLoop(); | ||
if (!loop.initialized()) return false; |
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.
Should we set an exception here?
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.
No.
We've had already thrown an exception inside PyEventLoop::getRunningLoop()
https://github.com/Distributive-Network/PythonMonkey/blob/4da2838/src/PyEventLoop.cc#L76-L77
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.
Nice work - can't wait to see this land
'no-empty': [ 'warn' ], | ||
'no-multi-spaces': [ 'off' ], | ||
'prettier/prettier': [ 'off' ], | ||
'vars-on-top': [ 'error' ], | ||
'no-var': [ 'off' ], | ||
'spaced-comment': [ 'warn' ], | ||
'brace-style': [ 'off' ], |
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.
note that this will generate false positives on code which acceptable per DCP JS coding style. Not really a big deal unless you want to never have spurious warnings. One of the core team members apparently made an es-lint plug-in that understands this, but I haven't reviewed it.
DCP bracing style is
- allman style for flow control and function declaration
- k&r style for object and function literals
- literal braces always followed by ; or ,
- top-level assignment to function literal in allman style, eg. module exports property assignments
This is a good style for reading but a hard style for linting.
setTimeout
/clearTimeout
in JS with C++ internal bindings, cleaner codecode
string tosetTimeout
as the callback function(separate PR)setInterval
/clearInterval
(separate PR)queueMicrotask