Closed
Bug 619446
Opened 15 years ago
Closed 15 years ago
Add a "LoadTracker" class
Categories
(Tree Management Graveyard :: TBPL, defect)
Tree Management Graveyard
TBPL
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: mstange, Assigned: mstange)
Details
Attachments
(1 file)
|
6.86 KB,
patch
|
Swatinem
:
review+
|
Details | Diff | Splinter Review |
This will simplify some things in the future.
Attachment #497863 -
Flags: review?(arpad.borsos)
|
||
Comment 1•15 years ago
|
||
Comment on attachment 497863 [details] [diff] [review]
v1
>+ this._statusCallback({
>+ loadpercent: this._numFinishedLoads / this._numTrackedLoads
>+ });
>+ },
>+};
Pleasy drop the trailing comma.
Also I hope you will soon add a user for addMorePotentialLoads, but I should perhaps read all the other review requests first before I accuse you of introducing unused code :P
Attachment #497863 -
Flags: review?(arpad.borsos) → review+
|
Assignee | |
Comment 2•15 years ago
|
||
(In reply to comment #1)
> Comment on attachment 497863 [details] [diff] [review]
> v1
>
> >+ this._statusCallback({
> >+ loadpercent: this._numFinishedLoads / this._numTrackedLoads
> >+ });
> >+ },
> >+};
>
> Pleasy drop the trailing comma.
Actually, I'm slowly going to convert everything to trailing commas since they make diffs shorter. Why don't you like them?
> Also I hope you will soon add a user for addMorePotentialLoads, but I should
> perhaps read all the other review requests first before I accuse you of
> introducing unused code :P
It's not used in any of the patches I've posted up to now, but it *is* being used somewhere in the giant patch I'm dissecting, so it will be used in a patch I'll post later on.
|
|
||
Comment 3•15 years ago
|
||
(In reply to comment #2)
> > Pleasy drop the trailing comma.
>
> Actually, I'm slowly going to convert everything to trailing commas since they
> make diffs shorter. Why don't you like them?
I simply consider it good style. After all, more strict languages throw a compile error for such syntax.
What about leading commas like so?:
, method: function class_method() {
}
|
|
Assignee | |
Comment 4•15 years ago
|
||
(In reply to comment #3)
> After all, more strict languages throw a
> compile error for such syntax.
I'm not convinced ;)
That's like saying I need to include <head> tags in HTML because they're required in XHTML.
> What about leading commas like so?:
>
> , method: function class_method() {
> }
Sorry, no, I can't believe you're even suggesting this.
:-D
|
|
||
Comment 5•15 years ago
|
||
Well than lets stop those syntax battles :D
There are some places inside mozilla where leading commas are used, like for shorthand constructurs.
|
|
Assignee | |
Comment 6•15 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
|
||
Updated•11 years ago
|
Product: Webtools → Tree Management
|
|
||
Updated•11 years ago
|
Product: Tree Management → Tree Management Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•