Code review comment for lp://staging/~bcsaller/juju-gui/svgdefs

LGTM with one ignorable and one mostly trivial.

https://codereview.appspot.com/11343043/diff/1/app/views/topology/service.js
File app/views/topology/service.js (right):

https://codereview.appspot.com/11343043/diff/1/app/views/topology/service.js#newcode990
app/views/topology/service.js:990: }
<shrug> above 7 lines could be the following, which reads as well or
better to me FWIW.

if (securityBadge) {
   landscapeAsset = '#landscape-restart';
} else if (rebootBadge) {
   landscapeAsset = '#landscape-security';
}

Alternatively 12 lines could be following.

if (landscape.getLandscapeBadge(d.model, 'reboot', 'round')) {
   landscapeAsset = '#landscape-restart';
} else if (
   landscape.getLandscapeBadge(d.model, 'security', 'round')) {
   landscapeAsset = '#landscape-security';
}

As I said, <shrug>, trivial, do what you want, etc.

https://codereview.appspot.com/11343043/diff/1/app/views/topology/service.js#newcode1010
app/views/topology/service.js:1010: existing.attr('xlink:href',
landscapeAsset);
Shouldn't this attr be defined in the "if (existing.empty())" branch
too? It looks like you can simply remove the enclosing "else."

https://codereview.appspot.com/11343043/