[Webkit-unassigned] [Bug 170932] Introduce 'commit_group' concept to be able to distinguish between commits with same revision but from different groups.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Apr 18 00:12:46 PDT 2017


https://bugs.webkit.org/show_bug.cgi?id=170932

--- Comment #2 from Ryosuke Niwa <rniwa at webkit.org> ---
Comment on attachment 307351
  --> https://bugs.webkit.org/attachment.cgi?id=307351
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.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20170418/336f4254/attachment.html>


More information about the webkit-unassigned mailing list