Skip to content

fix mTimerIdsToTimers remove error #8966

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

fix mTimerIdsToTimers remove error #8966

wants to merge 2 commits into from

Conversation

ASCE1885
Copy link
Contributor

In Timing.java, the key provided to the remove function of mTimerIdsToTimers is not correct, that may introduce bugs.

@ghost
Copy link

ghost commented Jul 22, 2016

By analyzing the blame information on this pull request, we identified @janicduplessis and @astreet to be potential reviewers.

@ghost
Copy link

ghost commented Jul 22, 2016

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at [email protected]. Thanks!

@ghost ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 22, 2016
@ghost
Copy link

ghost commented Jul 22, 2016

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@astreet
Copy link
Contributor

astreet commented Jul 22, 2016

Eek, great catch! Thanks! Amazing that Java doesn't error on this kind of thing...

@facebook-github-bot shipit

@ghost ghost added GH Review: accepted Import Started This pull request has been imported. This does not imply the PR has been approved. and removed GH Review: review-needed labels Jul 22, 2016
@ghost
Copy link

ghost commented Jul 22, 2016

Thanks for importing. If you are an FB employee go to Phabricator to review internal test results.

@ghost ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 22, 2016
@ide
Copy link
Contributor

ide commented Jul 22, 2016

@astreet what do you think about renaming the variable to mExecutorTokensToTimers?

@ghost ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 22, 2016
@ghost ghost closed this in 37e5f3b Jul 22, 2016
mpretty-cyro pushed a commit to HomePass/react-native that referenced this pull request Aug 25, 2016
@ASCE1885
fix mTimerIdsToTimers remove error
3cfd4cb 
Summary:
In Timing.java, the key provided to the remove function of mTimerIdsToTimers is not correct, that may introduce bugs.
Closes facebook#8966

Differential Revision: D3605291

Pulled By: astreet

fbshipit-source-id: 97563b6846e8f3f40d20b48b3852dd557c9932f3
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Import Started This pull request has been imported. This does not imply the PR has been approved.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ASCE1885 @astreet @ide