Refactor webkitTrac to Dashboard.Repository.OpenSource.trac and internalTrac to Dashboard.Repository.Internal.trac
Created attachment 258561 [details] Patch
Attachment 258561 [details] did not pass style-queue: ERROR: Tools/ChangeLog:1: ChangeLog entry has no bug number [changelog/bugnumber] [5] Total errors found: 1 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 258562 [details] Patch
Comment on attachment 258562 [details] 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.
Comment on attachment 258562 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=258562&action=review > Tools/ChangeLog:10 > + (BuildbotQueueView): Moved webkitTrac to Dashboard.Repository.OpenSource.trac and internalTrac to > + Dashboard.Repository.Internal.trac Very minor: "moved" is not the best way to describe the substitution you are performing. Also, we should add a period at the end of the sentence.
Created attachment 258759 [details] Patch
Comment on attachment 258759 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=258759&action=review > Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/BuildbotQueueView.js:80 > + webkitTrac = Dashboard.Repository.OpenSource.trac; > + internalTrac = Dashboard.Repository.Internal.trac; These lines are declaring global variables named webkitTrac and internalTrac. Please make these local variables by explicitly adding the keyword var before the name of the variable such that these lines read: var webkitTrac = Dashboard.Repository.OpenSource.trac; var internalTrac = Dashboard.Repository.Internal.trac; > Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/BuildbotQueueView.js:159 > + webkitTrac = Dashboard.Repository.OpenSource.trac; > + internalTrac = Dashboard.Repository.Internal.trac; Ditto.
Created attachment 258762 [details] Patch
Comment on attachment 258762 [details] Patch Clearing flags on attachment: 258762 Committed r188304: <http://trac.webkit.org/changeset/188304>
All reviewed patches have been landed. Closing bug.