Closed Bug 723937 Opened 10 years ago Closed 10 years ago

Make sure all the obviously private properties in Tilt start with an underscore

Categories

(DevTools :: Inspector, defect)

12 Branch
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 13

People

(Reporter: vporof, Assigned: vporof)

References

Details

(Whiteboard: [tilt])

Attachments

(1 file, 1 obsolete file)

Even if this rather useless "refactoring" may seem like a waste of time, right now there are some horrible inconsistencies across Tilt's codebase, which may cause confusions. Also, plenty things are obviously not to be messed with, like functions or flags used only for tests, so it would be a good idea to clean those up at some point.
Assignee: nobody → vporof
Status: NEW → ASSIGNED
Whiteboard: [tilt]
Depends on: 723588
Attached patch v1 (obsolete) — Splinter Review
Apart from the symmetrical chunks of lines where properties are now starting with an underscore, I moved a "let finalize = ..." function declaration outside from T_destroy to keep it as clean as possible, moved the LayoutHelpers import into a temp scope from one of the tests and fixed some javascript errors triggered only when running the arcball-reset tests.
Attachment #594951 - Flags: review?(rcampbell)
Attached patch v1.1Splinter Review
It seems Dão already made the LayoutHelpers change, which will make a hunk fail for the v1 patch. Updated.
Attachment #594956 - Flags: review?(rcampbell)
Attachment #594951 - Flags: review?(rcampbell)
Comment on attachment 594956 [details] [diff] [review]
v1.1

+    // if the visualizer is destroyed or destroying, don't do anything
+    if (!this.visualizers[aId] || this._isDestroying) {
+      return;
+    }

this sounds like a little extra from just adding some underscores to method names. Were you getting races on shutdown?

The rest looks as expected so r+ with a try run.
Attachment #594956 - Flags: review?(rcampbell) → review+
Whiteboard: [tilt] → [tilt][land-pending-try]
yes, I am dumb for not reading comment #1. I inferred it from the code ALL BY MYSELF!
https://tbpl.mozilla.org/?tree=Try&rev=843a6a70c862
Whiteboard: [tilt][land-pending-try] → [tilt]
Blocks: 726634
Whiteboard: [tilt] → [tilt][land-in-fx-team]
Attachment #594951 - Attachment is obsolete: true
https://hg.mozilla.org/integration/fx-team/rev/e1658e01d6c4
Whiteboard: [tilt][land-in-fx-team] → [tilt][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/e1658e01d6c4
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [tilt][fixed-in-fx-team] → [tilt]
Target Milestone: --- → Firefox 13
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.