[Webkit-unassigned] [Bug 147043] Remove hardcoded "internal" from Buildbot.prototype._normalizeQueueInfo.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Jul 21 10:41:52 PDT 2015


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

--- Comment #7 from Jason Marcell <jmarcell at apple.com> ---
Comment on attachment 257148
  --> https://bugs.webkit.org/attachment.cgi?id=257148
Remove hardcoded "internal" from Buildbot.prototype._normalizeQueueInfo.

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

>> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/Buildbot.js:110
>> +        queueInfo.branch = queueInfo.combinedQueues ?  null : queueInfo.branch || this.defaultBranches;
> 
> Why can't a combined queue be on a branch? We don't have any now, but I don't see any intrinsic reason for this limitation.
> 
> The operator precedence here seems non-obvious enough that I'd have appreciated parentheses.

It seems that a combined queue is just a collection of sub-queues and that information pertaining to the repository/branch, etc. of the individual queues would be better kept at the sub-queue level. Especially in the case where the individual sub-queues have different repositories, what would it mean for the containing combined queue to have a repository associated with it?

As Dan Bates mentioned to me, the parentheses are not necessary according to this: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Operator_Precedence
However, I agree that a set of parentheses might be nice to emphasize which part is evaluated first for someone who does not have this reference handy and does not have the operator precedence memorized.

>> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/BuildbotCombinedQueueView.js:28
>> +    var all_branch = {};
> 
> "all_branch" is neither named according to WebKit style, nor grammatically correct.

I agree that this is not grammatically correct. This was not my initial choice for variable name, but I wanted to remain consistent with the choice of "branch" on the "BuildbotQueue" object (a data structure that contains repository to branch mappings). Would "all_branches" be a better choice?

-- 
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/20150721/53eaabd0/attachment.html>


More information about the webkit-unassigned mailing list