558445 - harness.js cleanup

Status: RESOLVED WONTFIX

(Add-on SDK Graveyard :: General, defect)

Description

[Dietrich Ayala (:dietrich)]

Attachment: v1

This patch makes little if no functional change to the code in harness.js, and still passes all tests when I do "cfx testall".

Primarily it re-arranges the code into a structure that:

- is easier to follow, easing bug fixing
- removes use of globals within class definitions
- removes a few cycles found (still leaking though, so more work to do)

It still needs more cleanup and lots more comments. I'll keep working on it as I push on bug 557121.

Atul: Because of all the moving around I did, the diff is un-followable! I recommend applying the patch and reviewing from there :)

Comment 1

[Dietrich Ayala (:dietrich)]

Comment on attachment 438162 [details] [diff] [review]
v1

canceling review request, need a few more fixes. turns out "testall" doesn't flex some codepaths that user-created modules do.

Comment 2

[Dietrich Ayala (:dietrich)]

Attachment: v2

breaks development mode (which seems broken to begin with actually?).

Comment 3

[Dietrich Ayala (:dietrich)]

I've confirmed that my change prevents development mode from starting up correctly.

I've also confirmed that I don't know how to use dev mode.. I start it up, open a separate SDK environment, and make calls with -r, and nothing happens in either shell.

Comment 4

[Atul Varma [:atul]]

Nope, it sounds like dev-mode is indeed broken to begin with, not that you don't know how to use it--I'll look into this asap, thanks!

Comment 5

[Atul Varma [:atul]]

Just added bug 562397 to address the 'cfx develop' problem.

Comment 6

[Dietrich Ayala (:dietrich)]

Attachment: v2.1

minor fixes, cleanup

Comment 7

[Dietrich Ayala (:dietrich)]

patch is long-rotted. was mostly stylistic changes, so not a major loss.

Comment 8

[Myk Melez [:myk] [@mykmelez]]

The Add-on SDK is no longer a Mozilla Labs experiment and has become a big enough project to warrant its own Bugzilla product, so the "Add-on SDK" product has been created for it, and I am moving its bugs to that product.

To filter bugmail related to this change, filter on the word "looptid".