launchpad-reviewers team mailing list archive

["j.c.sackett" <jcsackett@xxxxxxxxxxxxx>]

> #10
> This would probably work out better as an attribute. Then you could use a
> valueFn() to set the lp_client. In this way passing a custom lp.client into
> initializer would automatically set it vs doing the same logic in the
> initializer itself.

Done.

> #73
> I think it'd be nice to have the html either part of a <script
> type="text/x-template"> and pulled in via setUp methods. This was we can keep
> all test dom in one container that's easy to .empty() and makes for more
> stable tests. You can be sure the tests aren't polluting the dom. I've been
> running into some of these in my quest to update to YUI 3.5.

This seems unnecessary at this stage; none of the tests actually modify the DOM, they just grab bits of it in config. However, I've made a note of this for a subsequent branch.

> #104
> I'd say to go ahead and remove the empty setup/teardown, but if you implement
> ^^ it'll make it necessary. What might actually be best is to have a second
> test suite that's working on the DOM state so that setup/teardown aren't doing
> work not required for the test_library_exists.
> 
> I've mocked up the changes here:
> https://pastebin.canonical.com/69284/
> 
> I've made changed the call to the constructor to just pass the config object
> inline vs a new variable config. No sense creating variables not required,
> extra memory, etc.

Setting aside the parts around x-template, I've made these changes. Thanks.
-- 
https://code.launchpad.net/~jcsackett/launchpad/add-yui-tests-for-comments/+merge/113118
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.