<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#c16">Comment # 16</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:jmarcell@apple.com" title="Jason Marcell <jmarcell@apple.com>"> <span class="fn">Jason Marcell</span></a>
</span></b>
<pre>(In reply to <a href="show_bug.cgi?id=152345#c9">comment #9</a>)
<span class="quote">> (In reply to <a href="show_bug.cgi?id=152345#c7">comment #7</a>)
> > Comment on <span class="bz_obsolete"><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>
> >
> > Looks good overall. Few things, but otherwise nice work... I know a lot of
> > people will be happy to see these changes merged. :)
> >
> > Also, thanks for adding QUnit! It's awesome that we have a way to test our
> > front-end now.
> >
> > > Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/BuildbotQueue.js:301
> > > var sortedRepositories = Dashboard.sortedRepositories;
> > > for (var i = 0; i < sortedRepositories.length; ++i) {
> > > var repositoryName = sortedRepositories[i].name;
> > > - 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;
> > > + var result = trac.compareRevisions(b.revision[repositoryName], a.revision[repositoryName]);
> > > if (result)
> > > return result;
> > > }
> >
> > Could compareIterations and compareIterationsByRevisions share this piece of
> > code? It looks identical.
>
> Yes, both compareIterations and compareIterationsByRevisions are very
> similar. The latter was introduced here:
> <a class="bz_bug_link
bz_status_RESOLVED bz_closed"
title="RESOLVED FIXED - build.webkit.org/dashboard still shows obsolete results for out of order builds sometimes"
href="show_bug.cgi?id=143885">https://bugs.webkit.org/show_bug.cgi?id=143885</a>
>
> We should probably find a way to collapse these into a single function if
> possible so that we are not duplicating code.</span >
I have addressed this in its own patch. Please see the following:
<a href="https://bugs.webkit.org/attachment.cgi?id=268503&action=review">https://bugs.webkit.org/attachment.cgi?id=268503&action=review</a></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>