Bug 153330

Summary: Fix bugs caused by incorrect usage of "branch" vs. "branchName".
Product: WebKit Reporter: Jason Marcell <jmarcell>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: adele, ap, dbates, dean_johnson, dino, jmarcell, lforschler
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=152345
Bug Depends on:    
Bug Blocks: 152345    
Attachments:
Description Flags
Patch
none
Patch dbates: review+

Jason Marcell
Reported 2016-01-21 16:22:42 PST
Fix bugs caused by incorrect usage of "branch" vs. "branchName".
Attachments
Patch (10.57 KB, patch)
2016-01-21 16:31 PST, Jason Marcell 		no flags
DetailsFormatted DiffDiff
Patch (10.62 KB, patch)
2016-01-22 11:09 PST, Jason Marcell 		dbates: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Jason Marcell
Comment 1 2016-01-21 16:31:52 PST
Created attachment 269517 [details] Patch
Jason Marcell
Comment 2 2016-01-22 11:09:21 PST
Created attachment 269586 [details] Patch
Dean Johnson
Comment 3 2016-01-27 10:43:19 PST
Comment on attachment 269586 [details] Patch r=me!
Daniel Bates
Comment 4 2016-01-27 13:50:25 PST
Comment on attachment 269586 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=269586&action=review > Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/BuildbotQueueView.js:113 > + _popoverLinesForCommitRange: function(trac, branch, firstRevisionNumber, lastRevisionNumber) I'm assuming you plan to make more use of the branch object in a subsequent patch than to get the branch name. Currently this function only makes use of the branch name. > Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/tests/tests.js:115 > + var branch = this.trunkBranch; This variable is used exactly once and I do not feel that its name makes the purpose of this variable any more understandable than its values. I suggest that we inline the value of this variable into the line below (line 116) and remove this variable. > Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/tests/tests.js:129 > + var branch = this.trunkBranch; By a similar argument as above, I suggest inlining the value of this variable where its referenced and remove this variable.
Jason Marcell
Comment 5 2016-01-27 13:57:02 PST
Comment on attachment 269586 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=269586&action=review >> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/BuildbotQueueView.js:113 >> + _popoverLinesForCommitRange: function(trac, branch, firstRevisionNumber, lastRevisionNumber) > > I'm assuming you plan to make more use of the branch object in a subsequent patch than to get the branch name. Currently this function only makes use of the branch name. That's correct. I do have a forthcoming patch that will make use of properties other than the branch name. >> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/tests/tests.js:115 >> + var branch = this.trunkBranch; > > This variable is used exactly once and I do not feel that its name makes the purpose of this variable any more understandable than its values. I suggest that we inline the value of this variable into the line below (line 116) and remove this variable. Will do. >> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/tests/tests.js:129 >> + var branch = this.trunkBranch; > > By a similar argument as above, I suggest inlining the value of this variable where its referenced and remove this variable. Will do.
Jason Marcell
Comment 6 2016-01-27 14:05:43 PST
Landed: http://trac.webkit.org/changeset/195690
Note You need to log in before you can comment on or make changes to this bug.