Summary: | Introduce 'commit_group' concept to be able to distinguish between commits with same revision but from different groups. | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | dewei_zhu | ||||
Component: | New Bugs | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED WONTFIX | ||||||
Severity: | Normal | CC: | dewei_zhu, rniwa | ||||
Priority: | P2 | ||||||
Version: | WebKit Nightly Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
dewei_zhu
2017-04-17 23:25:30 PDT
Created attachment 307351 [details]
Patch
Comment on attachment 307351 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=307351&action=review > Websites/perf.webkit.org/ChangeLog:12 > + CREATE TABLE commit_groups ( > + commit_group_id serial PRIMARY KEY, > + commit_group_name varchar(64), > + CONSTRAINT commit_group_name_must_be_unique UNIQUE(commit_group_name)); I think we should just add a new column to commits which is varchar(64) instead of adding a new table. > Websites/perf.webkit.org/ChangeLog:23 > + Nit: Whitespace. > Websites/perf.webkit.org/public/api/commits.php:29 > + $group = array_get($_GET, 'group'); > + $commits = $fetcher->fetch_between($repository_id, $preceding_revision, $last_revision, $group, $keyword); We need to make the JSON API's parameters consistent. Here, group is assumed to be an ID. > Websites/perf.webkit.org/public/api/commits.php:47 > + $group_name = array_get($_GET, 'group'); > + if ($group_name) > + $commits = $fetcher->fetch_last_reported_for_commit_group($repository_id, $group_name); And it's assumed to be a string. > Websites/perf.webkit.org/public/api/report-commits.php:42 > + > + Nit: two spaces. > Websites/perf.webkit.org/public/api/report-commits.php:104 > - $inserted_commit_id = $db->update_or_insert_row('commits', 'commit', array('repository' => $repository_id, 'revision' => $data['revision']), $data); > + $inserted_commit_id = $db->update_or_insert_row('commits', 'commit', array('repository' => $repository_id, 'revision' => $data['revision'], 'group' => $data['group']), $data); This is problematic because the existing database rows would have group being set to null. So this results in all macOS & iOS builds being inserted again. Also, /api/report doesn't set group name so this might end up creating a new row as well. Didn't we conclude that this approach was bad? |