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

Status: RESOLVED FIXED

(DevTools :: Inspector, defect)

Description

[Victor Porof [:vporof][:vp]]

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.

Comment 1

[Victor Porof [:vporof][:vp]]

Attachment: v1

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.

Comment 2

[Victor Porof [:vporof][:vp]]

Attachment: v1.1

It seems Dão already made the LayoutHelpers change, which will make a hunk fail for the v1 patch. Updated.

Comment 3

[Rob Campbell [:rc] (:robcee)]

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.

Comment 4

[Rob Campbell [:rc] (:robcee)]

yes, I am dumb for not reading comment #1. I inferred it from the code ALL BY MYSELF!

Comment 5

[Victor Porof [:vporof][:vp]]

https://tbpl.mozilla.org/?tree=Try&rev=843a6a70c862

Comment 6

[Rob Campbell [:rc] (:robcee)]

https://hg.mozilla.org/integration/fx-team/rev/e1658e01d6c4

Comment 7

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/e1658e01d6c4