[Webkit-unassigned] [Bug 168866] Commit should order by 'commit_order' as secondary key.

[bugzilla-daemon at webkit.org]

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

Ryosuke Niwa <rniwa at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #302736|review?                     |review+
              Flags|                            |

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

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20170225/74dd6a00/attachment.html>