Summary: | Should not mark a platform as missing in summary page if all expecting metrics are exlucded. | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | dewei_zhu | ||||||
Component: | New Bugs | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | commit-queue, dewei_zhu, rniwa, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
dewei_zhu
2017-09-14 17:30:41 PDT
Created attachment 320853 [details]
Patch
Comment on attachment 320853 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=320853&action=review > Websites/perf.webkit.org/public/v3/pages/summary-page.js:265 > var foundInSomeMetric = false; > + var excludedMerticCount = 0; Use let. > Websites/perf.webkit.org/public/v3/pages/summary-page.js:281 > - if (!foundInSomeMetric) > + if (!foundInSomeMetric && excludedMerticCount < metrics.length) Now we don't need foundInSomeMetric. You can just check excludedMerticCount < metrics.length since that condition also implies metrics.length. Comment on attachment 320853 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=320853&action=review >> Websites/perf.webkit.org/public/v3/pages/summary-page.js:265 >> + var excludedMerticCount = 0; > > Use let. Should I replace all usages of 'var' in this function? >> Websites/perf.webkit.org/public/v3/pages/summary-page.js:281 >> + if (!foundInSomeMetric && excludedMerticCount < metrics.length) > > Now we don't need foundInSomeMetric. You can just check excludedMerticCount < metrics.length > since that condition also implies metrics.length. I don't think we can do this. excludedConfigurations[platform.id()] just specifies the expected missing combinations for a platform. So it could be a subset of metrics. In this case, removing foundInSomeMetric could result in marking a platform missing incorrectly. Comment on attachment 320853 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=320853&action=review >>> Websites/perf.webkit.org/public/v3/pages/summary-page.js:281 >>> + if (!foundInSomeMetric && excludedMerticCount < metrics.length) >> >> Now we don't need foundInSomeMetric. You can just check excludedMerticCount < metrics.length >> since that condition also implies metrics.length. > > I don't think we can do this. > excludedConfigurations[platform.id()] just specifies the expected missing combinations for a platform. So it could be a subset of metrics. In this case, removing foundInSomeMetric could result in marking a platform missing incorrectly. For 'excludedConfigurations[platform.id()] just specifies the expected missing combinations for a platform' I meant 'excludedConfigurations[platform.id()] just specifies the expected missing Metrics for a platform.' Created attachment 320977 [details]
Patch for landing
Comment on attachment 320977 [details] Patch for landing Clearing flags on attachment: 320977 Committed r222123: <http://trac.webkit.org/changeset/222123> All reviewed patches have been landed. Closing bug. |