-
Notifications
You must be signed in to change notification settings - Fork 886
Update metrics.rs #5215
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
Update metrics.rs #5215
Conversation
|
Metrics from the original issue show that the lower resolution is not being used. Not sure @michaelsproul if you need 5ms resolution, otherwise the metric lin range could be of resolution 10ms and range up to 200ms
|
yeah IMO 10ms would be fine, I would also prefer less buckets (10-15, not 20) |
@DoTheBestToGetTheBest you also need to accept the CLA |
thank you for your review!
thank you for your review! changed to 10 |
what is this? |
It's a contributor license agreement, it just says that you agree to license your contributions under the same license as the rest of Lighthouse (Apache 2.0). It's a bit of a bureaucratic hurdle, but only takes a second, see: #5215 (comment). |
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.
Thanks! 15 should be ok
At the bottom of this PR, or the direct link: https://cla-assistant.io/sigp/lighthouse?pullRequest=5215 |
related with #5206
i adjusted for a range up to 0.1 seconds (100ms)