WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
193132
Refactor 'platforms' table to contain group information.
https://bugs.webkit.org/show_bug.cgi?id=193132
Summary
Refactor 'platforms' table to contain group information.
dewei_zhu
Reported
2019-01-03 21:10:42 PST
Refactor platform table to contain group infomation.
Attachments
Patch
(25.31 KB, patch)
2019-01-03 21:32 PST
,
dewei_zhu
no flags
Details
Formatted Diff
Diff
Patch
(25.31 KB, patch)
2019-01-03 21:34 PST
,
dewei_zhu
rniwa
: review+
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
dewei_zhu
Comment 1
2019-01-03 21:32:22 PST
Created
attachment 358307
[details]
Patch
dewei_zhu
Comment 2
2019-01-03 21:34:05 PST
Created
attachment 358308
[details]
Patch
Ryosuke Niwa
Comment 3
2019-01-10 21:04:54 PST
Comment on
attachment 358308
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=358308&action=review
> Websites/perf.webkit.org/public/admin/platforms.php:99 > + array_push($platform_group_options, array('platformgroup_id' => NULL, 'platformgroup_name' => '<No Group>'));
Maybe just say "-"?
> Websites/perf.webkit.org/public/admin/platforms.php:103 > + $id = $platform_row['platform_id'];
Call intval here just be safe.
> Websites/perf.webkit.org/public/v3/models/analysis-task.js:180 > + const fetchingMeasurementSetForOtherPlatformInSameGroupPromises = this._commitSetForOtherPlatformsInSameGroup();
This is awfully long variable name. Also, this isn't fetching measurement set so we probably shouldn't call it that. How about commitSetsInSamePlatformGroupPromise?
> Websites/perf.webkit.org/public/v3/models/analysis-task.js:198 > + _commitSetForOtherPlatformsInSameGroup()
"SameGroup" is a bit ambiguous here because there is also test group. I think I'd prefer call this _commitSetsInSamePlatformGroup.
dewei_zhu
Comment 4
2020-10-27 18:01:34 PDT
This should have been landed long time ago. Rebased against latest main branch and verified all unit tests are still passing. Landed in
r269083
.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug