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

Make SchedulerMinHeap flow strict #16351

Merged
merged 1 commit into from
Aug 10, 2019
Merged

Conversation

dmnd
Copy link
Contributor

@dmnd dmnd commented Aug 10, 2019

@acdlite while browsing Twitter, I saw an opportunity to do
something more productive than browsing Twitter.

Test plan:

yarn flow-ci, yarn test-prod, yarn lint

@sizebot
Copy link

sizebot commented Aug 10, 2019

No significant bundle size changes to report.

Generated by 🚫 dangerJS

@acdlite
Copy link
Collaborator

acdlite commented Aug 10, 2019

Thanks!

Super nit: Could you change the name of the argument to i and keep the other names the same? So that it’s symmetrical with parentIndex and so forth.

@acdlite while browsing Twitter, I saw [an opportunity][1] to do
something more productive than browsing Twitter.

[1]: https://twitter.com/acdlite/status/1160247965908234240

Test plan:

`yarn flow-ci`, `yarn test-prod`, `yarn lint`
@dmnd dmnd force-pushed the flow-strict-heap branch from ab9b104 to 77c34e5 Compare August 10, 2019 20:29
@dmnd
Copy link
Contributor Author

dmnd commented Aug 10, 2019

Sure, done.

Copy link
Collaborator

@acdlite acdlite left a comment

Choose a reason for hiding this comment

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

Thank you!

@acdlite acdlite merged commit 7a7e792 into facebook:master Aug 10, 2019
@dmnd dmnd deleted the flow-strict-heap branch August 10, 2019 20:55
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