Code review comment for lp://staging/~gary/juju-gui/bug1228166

On 2013/09/24 18:37:48, jeff.pihach wrote:
> Code LGTM with trivial and will QA and report back.

Thank you very much for review and qa!

https://codereview.appspot.com/13841044/diff/1/app/views/viewlets/unit-details.js
> File app/views/viewlets/unit-details.js (right):

https://codereview.appspot.com/13841044/diff/1/app/views/viewlets/unit-details.js#newcode40
> app/views/viewlets/unit-details.js:40: if (value) {
> if everything requires a value I would do:

> if (!value) { return; }

> just to avoid the indentation throughout the whole function (personal
> preference)

Yeah, I dunno. I changed it to match your preference this time. :-)

https://codereview.appspot.com/13841044/diff/1/app/views/viewlets/unit-details.js#newcode89
> app/views/viewlets/unit-details.js:89: 'get': function(model) {
> What is this for? Where is it used?

We discussed, and with Ben decided to move the functionality into
databinding and remove the extension feature I was using.

https://codereview.appspot.com/13841044/diff/1/test/test_unit_detail_viewlet.js
> File test/test_unit_detail_viewlet.js (right):

https://codereview.appspot.com/13841044/diff/1/test/test_unit_detail_viewlet.js#newcode67
> test/test_unit_detail_viewlet.js:67:
assert.strictEqual(node.getContent(), '');
> getContent() is deprecated in favour of getHTML()

Ah! Thanks, changed.

https://codereview.appspot.com/13841044/