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

Android: Increase RN-timers idling resources look-ahead to 1.5sec #1037

Merged
merged 3 commits into from
Nov 14, 2018

Conversation

d4vidi
Copy link
Collaborator

@d4vidi d4vidi commented Nov 11, 2018

@rotemmiz just for review, don't merge

@d4vidi d4vidi requested a review from rotemmiz November 11, 2018 17:44
@rotemmiz rotemmiz changed the title Increase RN-timers idling resources look-ahead to 1.5sec [WIP] Android: Increase RN-timers idling resources look-ahead to 1.5sec Nov 11, 2018
Copy link
Member

@rotemmiz rotemmiz left a comment

Choose a reason for hiding this comment

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

I merged with master and disable artifacts snapshot checks. Let's see what's the verdict.

final int intervalMS = nextTimerReflected.field(TIMER_FIELD_INTERVAL).get();
final boolean isRepetitive = nextTimerReflected.field(TIMER_FIELD_REPETITIVE).get();

// Log.i(LOG_TAG, "Next timer has duration of: " + intervalMS
Copy link
Member

Choose a reason for hiding this comment

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

Not sure those comments are necessary, unless there's a hack that needs an explanation.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Assuming you're referring to the comments and not the logs -
they're necessary because we didn't unit test, so there's nothing here to explain the idea behind the implementation. This will change soon, of course.

@d4vidi
Copy link
Collaborator Author

d4vidi commented Nov 14, 2018

@rotemmiz why not rebase over master, instead?

@d4vidi
Copy link
Collaborator Author

d4vidi commented Nov 14, 2018

Anyways this branch is becoming stale. If you're not comfortable with the code, let's try to think of smt else :)

@rotemmiz
Copy link
Member

We're going to squash it anyway, it doesn't matter. But build is still red.

@rotemmiz
Copy link
Member

Retriggerd the build, let's see

@d4vidi
Copy link
Collaborator Author

d4vidi commented Nov 14, 2018

Cool, tnx :)

@rotemmiz rotemmiz changed the title [WIP] Android: Increase RN-timers idling resources look-ahead to 1.5sec Android: Increase RN-timers idling resources look-ahead to 1.5sec Nov 14, 2018
@rotemmiz rotemmiz merged commit 5a179ca into master Nov 14, 2018
@rotemmiz rotemmiz deleted the timers-ir-1500-ms branch November 14, 2018 10:38
@lock lock bot locked as resolved and limited conversation to collaborators Nov 17, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants