Closed
Bug 868711
Opened 12 years ago
Closed 11 years ago
Needlessly confusing expression in _trackSlowStartup
Categories
(Firefox :: General, defect)
Firefox
General
Tracking
()
RESOLVED
FIXED
People
(Reporter: jruderman, Assigned: sshagarwal)
References
Details
(Whiteboard: [good first bug][mentor=gavin])
Attachments
(1 file, 2 obsolete files)
|
(deleted),
patch
|
sshagarwal
:
review+
|
Details | Diff | Splinter Review |
> http://mxr.mozilla.org/mozilla-central/source/browser/components/nsBrowserGlue.js#453
> averageTime = (averageTime * samples + currentTime) / ++samples;
Is the first use of "samples" affected by the pre-increment or not? I had to try it out to know.
|
||
Updated•11 years ago
|
Whiteboard: [good first bug][mentor=gavin]
|
Assignee | |
Comment 1•11 years ago
|
||
Attachment #788984 -
Flags: review?(gavin.sharp)
|
|
||
Comment 2•11 years ago
|
||
I think I prefer my suggested version on IRC :) As it is, re-using averageTimes to temporarily hold a value that isn't at all the average time is a bit confusing.
|
|
||
Comment 3•11 years ago
|
||
Attachment #788984 -
Flags: review?(gavin.sharp) → feedback+
|
|
Assignee | |
Comment 4•11 years ago
|
||
Attachment #788984 -
Attachment is obsolete: true
Attachment #789004 -
Flags: review?(gavin.sharp)
|
|
Assignee | |
Updated•11 years ago
|
Assignee: nobody → syshagarwal
Status: NEW → ASSIGNED
|
|
||
Comment 5•11 years ago
|
||
Comment on attachment 789004 [details] [diff] [review]
Patch v2
minor nit: I intentionally changed that first line to:
let totalTimes = (averageTime * samples) + currentTime;
because I don't like having to remember PEDMAS :) (also otherwise the parentheses are unnecessary).
r=me with that change
Attachment #789004 -
Flags: review?(gavin.sharp) → review+
|
|
Assignee | |
Comment 6•11 years ago
|
||
Okay, done.
Thanks!
Carrying over review from Gavin.
Attachment #789004 -
Attachment is obsolete: true
Attachment #789107 -
Flags: review+
|
|
Assignee | |
Updated•11 years ago
|
Keywords: checkin-needed
|
||
Comment 7•11 years ago
|
||
Thanks for the patch, Suyash!
https://hg.mozilla.org/integration/fx-team/rev/7c461e6aea7f
Keywords: checkin-needed
|
|
Assignee | |
Updated•11 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
|
||
Comment 8•11 years ago
|
||
You need to log in
before you can comment on or make changes to this bug.
Description
•