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

Rename Scheduler methods more accurately #12770

Merged
merged 4 commits into from
May 16, 2018

Conversation

flarnie
Copy link
Contributor

@flarnie flarnie commented May 9, 2018

what is the change?:

rIC -> scheduleCallback

We will later expose a second method for different priority level, name
TBD. Since we only have one priority right now we can delay the
bikeshedding about the priority names.

cIC -> cancelScheduledCallback

This method can be used to cancel callbacks scheduled at any priority
level, and will remain named this way.

why make this change?:
Originally this module contained a polyfill for requestIdleCallback
and cancelIdleCallback but we are changing the behavior so it's no
longer just a polyfill. The new names are more semantic and distinguish
this from the original polyfill functionality.

test plan:
Ran the tests

why make this change?:
Getting this out of the way so things are more clear.

Coming Up Next:

  • Switching from a Map of ids and an array to a linked list for storing
    callbacks.
  • Error handling

@sebmarkbage
Copy link
Collaborator

sebmarkbage commented May 9, 2018

Nit: In my mind the "callback" is the type signature but it doesn't say anything about the nature of the callback. It's like saying printString(...) instead of printWarning(...). What is this thing we're scheduling supposed to do? Work?

flarnie added 3 commits May 15, 2018 14:56
**what is the change?:**
```
rIC -> scheduleCallback
```
We will later expose a second method for different priority level, name
TBD. Since we only have one priority right now we can delay the
bikeshedding about the priority names.

cIC -> cancelScheduledCallback
This method can be used to cancel callbacks scheduled at any priority
level, and will remain named this way.

why make this change?:
Originally this module contained a polyfill for requestIdleCallback
and cancelIdleCallback but we are changing the behavior so it's no
longer just a polyfill. The new names are more semantic and distinguish
this from the original polyfill functionality.

**test plan:**
Ran the tests

**why make this change?:**
Getting this out of the way so things are more clear.

**Coming Up Next:**
- Switching from a Map of ids and an array to a linked list for storing
callbacks.
- Error handling
@flarnie flarnie force-pushed the improveSchedulerMethodNames branch from 719d3b2 to c294700 Compare May 15, 2018 22:00
@@ -1,4 +1,4 @@
/**
**
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops

@flarnie
Copy link
Contributor Author

flarnie commented May 15, 2018

This should be an easy review~
waiting_puppies_ping

@flarnie
Copy link
Contributor Author

flarnie commented May 16, 2018

thanks_dog

@flarnie flarnie merged commit ef294ed into facebook:master May 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants