Code review comment for lp://staging/~bcsaller/juju-gui/service-click-actions-begone

Revision history for this message
Nicola Larosa (teknico) wrote :

Land as is.

See also some comments below.

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

https://codereview.appspot.com/7228070/diff/3001/app/views/topology/service.js#newcode94
app/views/topology/service.js:94: // with this module as self and a box
as its argument.
Why is it called "d" if it's a box?

https://codereview.appspot.com/7228070/diff/3001/app/views/topology/service.js#newcode195
app/views/topology/service.js:195: self.showService(box);
And here we have both a "d" and a "box". Confusing.

https://codereview.appspot.com/7228070/diff/3001/app/views/topology/service.js#newcode203
app/views/topology/service.js:203: destroyServiceClick: function(data,
self) {
Oh, so "d" means "data".

https://codereview.appspot.com/7228070/diff/3001/app/views/topology/service.js#newcode906
app/views/topology/service.js:906: showService: function(d) {
The parameter is intended to be called "box", according to the comment
block right above, and it's probably a good idea.

https://codereview.appspot.com/7228070/diff/3001/test/test_application_notifications.js
File test/test_application_notifications.js (right):

https://codereview.appspot.com/7228070/diff/3001/test/test_application_notifications.js#newcode240
test/test_application_notifications.js:240: // The _destroyCallback args
are: service, view, confirm button, event.
Is this comment still valid?

https://codereview.appspot.com/7228070/

« Back to merge proposal