Bug 153467

Summary: Sort incoming commits via date instead of revision number
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
none
Patch
none
Patch ap: review+

Jason Marcell
Reported 2016-01-25 16:48:13 PST
Sort incoming commits via date instead of revision number
Attachments
Patch (1.88 KB, patch)
2016-01-25 16:53 PST, Jason Marcell 		no flags
DetailsFormatted DiffDiff
Patch (9.62 KB, patch)
2016-01-26 16:22 PST, Jason Marcell 		no flags
DetailsFormatted DiffDiff
Patch (9.62 KB, patch)
2016-01-26 17:04 PST, Jason Marcell 		no flags
DetailsFormatted DiffDiff
Patch (10.90 KB, patch)
2016-01-26 17:29 PST, Jason Marcell 		ap: review+
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Jason Marcell
Comment 1 2016-01-25 16:53:51 PST
Created attachment 269817 [details] Patch
Dean Johnson
Comment 2 2016-01-25 17:05:28 PST
Comment on attachment 269817 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=269817&action=review r=me w/ passing tests! :) > Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/Trac.js:200 > + this.recordedCommits = newCommits.concat(this.recordedCommits).sort(function(a, b) { return a.date - b.date; }); I'm assuming this is the case, but just to be sure: these dates are either stored as integers or are data types that have logic to handle arithmetic operations and give correct results? Actually, tests for this would completely eliminate my concern :)
Jason Marcell
Comment 3 2016-01-26 16:22:35 PST
Created attachment 269946 [details] Patch
Jason Marcell
Comment 4 2016-01-26 16:23:31 PST
(In reply to comment #2) > Comment on attachment 269817 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=269817&action=review > > r=me w/ passing tests! :) > > > Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/Trac.js:200 > > + this.recordedCommits = newCommits.concat(this.recordedCommits).sort(function(a, b) { return a.date - b.date; }); > > I'm assuming this is the case, but just to be sure: these dates are either > stored as integers or are data types that have logic to handle arithmetic > operations and give correct results? Yes, that is the case. > > Actually, tests for this would completely eliminate my concern :) I've added tests.
Dean Johnson
Comment 5 2016-01-26 16:54:43 PST
Comment on attachment 269946 [details] Patch Looks good to me!
Jason Marcell
Comment 6 2016-01-26 17:04:28 PST
Created attachment 269953 [details] Patch
Alexey Proskuryakov
Comment 7 2016-01-26 17:12:46 PST
Comment on attachment 269953 [details] Patch I don't think that it's right for the tests to live inside public_html, why did we put them there? The patch doesn't apply, but it looks fine to me.
Jason Marcell
Comment 8 2016-01-26 17:19:29 PST
(In reply to comment #7) > Comment on attachment 269953 [details] > Patch > > I don't think that it's right for the tests to live inside public_html, why > did we put them there? I'm definitely open to changing the way these unit tests are organized. I welcome feedback on a better organizational layout. If we come up with something better I can address it in a future bug/patch. > > The patch doesn't apply, but it looks fine to me. I can rebase the patch so that it applies cleanly.
Jason Marcell
Comment 9 2016-01-26 17:29:36 PST
Created attachment 269957 [details] Patch
Jason Marcell
Comment 10 2016-01-26 23:45:43 PST
Landed: http://trac.webkit.org/changeset/195658
Note You need to log in before you can comment on or make changes to this bug.