Closed eeeps closed 3 years ago
@rviscomi thank you so very, very much.
Since you reviewed...
I'll see if I can get another full review by EOD.
The resource selection logic LGTM.
Added a few more values to the reported JSON -- all stuff we were either already looking at in order to calculate other things, or the values of attributes we were previously only testing for the presense of.
There is some duplication with what's captured in images.js, but I think having those things here will make analysis that would have required a join less error-prone.
@rviscomi asked for some WPT results; here they are:
This looks great, thanks for the test cases. I'm comfortable merging this as-is in the interest of time. If there's any follow-up feedback or bug fixes we could open another PR.
Progress on https://github.com/HTTPArchive/almanac.httparchive.org/issues/2144
This is not quite ready to be merged...
It all works, but I need to:
Then there are some nice-to-haves... but that's the must do list.
PR'ing to get some more eyes on what's here now and comments in a place other than twitter.
My "test suite" such as it is is: https://codepen.io/eeeps/debug/Exmxwzq Here's a successful WPT run against that: https://webpagetest.org/custom_metrics.php?test=210629_AiDc5N_8d945980cad843b7ed362df6d53ef17e&run=1&cached=0