Bug 168866 – Commit should order by 'commit_order' as secondary key.

• Component: New Bugs
• Status: RESOLVED FIXED

Commit should order by 'commit_order' as secondary key.

Created attachment 302736 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 302736 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=302736&action=review

> Websites/perf.webkit.org/public/include/commit-log-fetcher.php:87
> -        return $this->format_single_commit($this->db->select_first_row('commits', 'commit', array('repository' => $repository_id), 'time'));
> +        return $this->format_single_commit($this->db->select_first_row('commits', 'commit', array('repository' => $repository_id), ['time', 'order']));

[] syntax doesn't work on an older versions of PHP.
So please use array() instead.

> Websites/perf.webkit.org/public/include/db.php:217
> +            if (!is_array($order_by)) {
> +                $order_by = array($order_by);
> +            }

Nit: No curtly braces around a single line statement.

> Websites/perf.webkit.org/public/include/db.php:223
> +                $order_column = $this->prefixed_name($order_key, $prefix);
> +                if ($descending_order)
> +                    $order_column .= ' DESC';

Might be cleaner to always specify ASC/DES using tertiary expression as in:
$order_column = $this->prefixed_name($order_key, $prefix) . ' ' . ($descending_order ? 'DESC' : 'ASC');

> Websites/perf.webkit.org/server-tests/api-commits.js:137
> +        it("should return the list of ordered commits for a given repository", () => {

Please also add tests for latest, & oldest, and last-reported.

> Websites/perf.webkit.org/server-tests/api-commits.js:146
> +                assert.equal(commits.length, 6);

You can just check against submittedCommits.length here.

> Websites/perf.webkit.org/server-tests/api-commits.js:147
> +                var submittedCommits = systemVersionCommits['commits'];

Use const instead of var.

Created attachment 302748 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 302748 [details]
Patch for landing

Clearing flags on attachment: 302748

Committed r213003: <http://trac.webkit.org/changeset/213003>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.