Update TTI scoring label to 5000ms (matches guidance) #947
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
brendankenny
requested changes
Nov 16, 2016
| @@ -17,6 +17,8 @@ const Formatter = require('../formatters/formatter'); | |||
| // https://www.desmos.com/calculator/jlrx14q4w8 | |||
| const SCORING_POINT_OF_DIMINISHING_RETURNS = 1700; | |||
| const SCORING_MEDIAN = 5000; | |||
| // This aligns with the external TTI targets in https://goo.gl/yXqxpL | |||
There was a problem hiding this comment.
Do we have a tracking issue for updating the scoring? Would be good to add here
There was a problem hiding this comment.
I've thrown together #948 but am happy to detail more on there if needed. Is that what you had in mind?
brendankenny
approved these changes
Nov 16, 2016
| @@ -17,6 +17,8 @@ const Formatter = require('../formatters/formatter'); | |||
| // https://www.desmos.com/calculator/jlrx14q4w8 | |||
| const SCORING_POINT_OF_DIMINISHING_RETURNS = 1700; | |||
| const SCORING_MEDIAN = 5000; | |||
| // This aligns with the external TTI targets in https://goo.gl/yXqxpL | |||
andrewrota
pushed a commit
to andrewrota/lighthouse
that referenced
this pull request
Jan 13, 2017
* Update TTI scoring label to 5000 (matches guidance) * Add ms units to the TTI included in reports
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The current TTI uses our point of diminishing returns as a label, however this number (1700) currently doesn't align with the external guidance on TTI targets (5s on 3G). This PR updates the TTI target to 5000 as discussed with sir @brendankenny.
A separate commit also adds the missing
msunits to the report that are used in other performance metrics but not included in TTI for some reason. Two fixes for the price of one 🐦 🐦