[Webkit-unassigned] [Bug 147805] Refactor webkitTrac to Dashboard.Repository.OpenSource.trac and internalTrac to Dashboard.Repository.Internal.trac

[bugzilla-daemon at webkit.org]

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

Daniel Bates <dbates at webkit.org> changed:

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

--- Comment #4 from Daniel Bates <dbates at webkit.org> ---
Comment on attachment 258562
  --> https://bugs.webkit.org/attachment.cgi?id=258562
Patch

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

Note, we'll need to make a corresponding change to our internal code before/after we land this change.

> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/BuildbotQueueView.js:97
> -        var latestRecordedOpenSourceRevisionNumber = webkitTrac.latestRecordedRevisionNumber;
> -        if (!latestRecordedOpenSourceRevisionNumber || webkitTrac.oldestRecordedRevisionNumber > latestProductiveIteration.revision[Dashboard.Repository.OpenSource.name]) {
> -            webkitTrac.loadMoreHistoricalData();
> +        var latestRecordedOpenSourceRevisionNumber = Dashboard.Repository.OpenSource.trac.latestRecordedRevisionNumber;
> +        if (!latestRecordedOpenSourceRevisionNumber || Dashboard.Repository.OpenSource.trac.oldestRecordedRevisionNumber > latestProductiveIteration.revision[Dashboard.Repository.OpenSource.name]) {
> +            Dashboard.Repository.OpenSource.trac.loadMoreHistoricalData();
>              return;
>          }
>  
>          // FIXME: To be 100% correct, we should also filter out changes that are ignored by
>          // the queue, see _should_file_trigger_build in wkbuild.py.
> -        var totalRevisionsBehind = webkitTrac.commitsOnBranch(queue.branch.openSource, function(commit) { return commit.revisionNumber > latestProductiveIteration.revision[Dashboard.Repository.OpenSource.name]; }).length;
> +        var totalRevisionsBehind = Dashboard.Repository.OpenSource.trac.commitsOnBranch(queue.branch.openSource, function(commit) { return commit.revisionNumber > latestProductiveIteration.revision[Dashboard.Repository.OpenSource.name]; }).length;
>  
>          if (latestProductiveIteration.revision[Dashboard.Repository.Internal.name]) {
> -            var latestRecordedInternalRevisionNumber = internalTrac.latestRecordedRevisionNumber;
> -            if (!latestRecordedInternalRevisionNumber || internalTrac.oldestRecordedRevisionNumber > latestProductiveIteration.revision[Dashboard.Repository.Internal.name]) {
> -                internalTrac.loadMoreHistoricalData();
> +            var latestRecordedInternalRevisionNumber = Dashboard.Repository.Internal.trac.latestRecordedRevisionNumber;
> +            if (!latestRecordedInternalRevisionNumber || Dashboard.Repository.Internal.trac.oldestRecordedRevisionNumber > latestProductiveIteration.revision[Dashboard.Repository.Internal.name]) {
> +                Dashboard.Repository.Internal.trac.loadMoreHistoricalData();
>                  return;
>              }
>  
> -            totalRevisionsBehind += internalTrac.commitsOnBranch(queue.branch.internal, function(commit) { return commit.revisionNumber > latestProductiveIteration.revision[Dashboard.Repository.Internal.name]; }).length;
> +            totalRevisionsBehind += Dashboard.Repository.Internal.trac.commitsOnBranch(queue.branch.internal, function(commit) { return commit.revisionNumber > latestProductiveIteration.revision[Dashboard.Repository.Internal.name]; }).length;
>          }

For your consideration, I suggest we define local variables webkitTrac :=  Dashboard.Repository.OpenSource.trac and internalTrac := Dashboard.Repository.Internal.trac. Then we do no need to modify all the existing lines in this function. Additionally, defining such local variables will likely make it more straightforward to refactor this code to work for arbitrary repositories in a future patch among other benefits.

> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/BuildbotQueueView.js:162
> -        var linesForOpenSource = this._popoverLinesForCommitRange(webkitTrac, queue.branch.openSource, latestProductiveIteration.revision[Dashboard.Repository.OpenSource.name] + 1, webkitTrac.latestRecordedRevisionNumber);
> +        var linesForOpenSource = this._popoverLinesForCommitRange(Dashboard.Repository.OpenSource.trac, queue.branch.openSource, latestProductiveIteration.revision[Dashboard.Repository.OpenSource.name] + 1, Dashboard.Repository.OpenSource.trac.latestRecordedRevisionNumber);
>          for (var i = 0; i != linesForOpenSource.length; ++i)
>              content.appendChild(linesForOpenSource[i]);
>  
>          var linesForInternal = [];
> -        if (latestProductiveIteration.revision[Dashboard.Repository.Internal.name] && internalTrac.latestRecordedRevisionNumber)
> -            var linesForInternal = this._popoverLinesForCommitRange(internalTrac, queue.branch.internal, latestProductiveIteration.revision[Dashboard.Repository.Internal.name] + 1, internalTrac.latestRecordedRevisionNumber);
> +        if (latestProductiveIteration.revision[Dashboard.Repository.Internal.name] && Dashboard.Repository.Internal.trac.latestRecordedRevisionNumber)
> +            var linesForInternal = this._popoverLinesForCommitRange(Dashboard.Repository.Internal.trac, queue.branch.internal, latestProductiveIteration.revision[Dashboard.Repository.Internal.name] + 1, Dashboard.Repository.Internal.trac.latestRecordedRevisionNumber);

Ditto.

-- 
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/20150811/21642d84/attachment.html>