<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - Teach dashboard code to compare non-integer revisions."
href="https://bugs.webkit.org/show_bug.cgi?id=152345#c8">Comment # 8</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - Teach dashboard code to compare non-integer revisions."
href="https://bugs.webkit.org/show_bug.cgi?id=152345">bug 152345</a>
from <span class="vcard"><a class="email" href="mailto:dbates@webkit.org" title="Daniel Bates <dbates@webkit.org>"> <span class="fn">Daniel Bates</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=267593&action=diff" name="attach_267593" title="Patch">attachment 267593</a> <a href="attachment.cgi?id=267593&action=edit" title="Patch">[details]</a></span>
Patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=267593&action=review">https://bugs.webkit.org/attachment.cgi?id=267593&action=review</a>
<span class="quote">> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/BuildbotQueue.js:276
> - var result = b.revision[repositoryName] - a.revision[repositoryName];
> + var trac = sortedRepositories[i].trac;
> + if (!trac || b.revision[repositoryName] === undefined || a.revision[repositoryName] === undefined)
> + return undefined;</span >
Can you give an example where we would not have a revision for a repository?
<span class="quote">> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/BuildbotQueue.js:298
> + var trac = sortedRepositories[i].trac;
> + if (!trac || b.revision[repositoryName] === undefined || a.revision[repositoryName] === undefined)
> + return undefined;
> + var result = trac.compareRevisions(b.revision[repositoryName], a.revision[repositoryName]);</span >
Ditto.
<span class="quote">> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/BuildbotQueueView.js:95
> + if (!trac.latestRecordedRevisionNumber || comparison === undefined || comparison > 0) {</span >
Can you give an example when comparison === undefined?
<span class="quote">> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/BuildbotQueueView.js:100
> + totalRevisionsBehind += trac.commitsOnBranch(branch.name, function(commit, index, arr) { return index > arr.objectIndexOf(latestProductiveRevisionNumber, "revisionNumber"); }).length;</span >
The name arr is not a good name for the array of commits. I am assuming arr is short for "array". Regardless, neither "arr" nor "array" are good names because the former is an abbreviation of the latter (and I do not find it more canonical than the word "array") and the latter is a generic term that does not convey anything about the contents of the array. Maybe a better name would be commits or allCommits?
<span class="quote">> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/BuildbotQueueView.js:145
> + var commits = trac.commitsOnBranch(branch.name, function(commit, index, arr) { return index >= arr.objectIndexOf(firstRevisionNumber, "revisionNumber") && index <= arr.objectIndexOf(lastRevisionNumber, "revisionNumber"); });</span >
Ditto.
<span class="quote">> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/Trac.js:68
> + return this.recordedCommits.filter(function(commit, index, arr) {</span >
Ditto.
<span class="quote">> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/Trac.js:266
> + var indexA = this.recordedCommits.objectIndexOf(a, "revisionNumber");
> + var indexB = this.recordedCommits.objectIndexOf(b, "revisionNumber");</span >
Can you elaborate on when a commit would not have a revision?
<span class="quote">> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/Trac.js:269
> + this.loadMoreHistoricalData();
> + return undefined;</span >
This does not seem like the correct approach. Having a comparison function fetch more data and have a polymorphic return type is weird.</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>