Code review comment for lp:~bcsaller/juju-gui/phantom-testing

Revision history for this message
Gary Poster (gary) wrote :

Trivial comments. Thank you for doing this! It looks good. I will
give it a try tomorrow morning.

https://codereview.appspot.com/7201049/diff/1/Makefile
File Makefile (right):

https://codereview.appspot.com/7201049/diff/1/Makefile#newcode336
Makefile:336:
trivial, but might as well revert this.

https://codereview.appspot.com/7201049/diff/1/app/app.js
File app/app.js (right):

https://codereview.appspot.com/7201049/diff/1/app/app.js#newcode199
app/app.js:199: // Concession to testing, they need muck
Trivial, but I suggest simply saying "PhantomJS handles the console
itself, if used."

https://codereview.appspot.com/7201049/diff/1/app/app.js#newcode648
app/app.js:648: //console.log('load service', evt);
Could simply delete IMO

https://codereview.appspot.com/7201049/diff/1/app/assets/javascripts/d3-components.js
File app/assets/javascripts/d3-components.js (right):

https://codereview.appspot.com/7201049/diff/1/app/assets/javascripts/d3-components.js#newcode319
app/assets/javascripts/d3-components.js:319: //console.debug('D3 Handler
for', selector, trigger);
Just delete it IMO

https://codereview.appspot.com/7201049/diff/1/app/assets/javascripts/d3-components.js#newcode497
app/assets/javascripts/d3-components.js:497: //console.log('Component',
methodName, 'on', name);
Just delete it IMO

https://codereview.appspot.com/7201049/diff/1/app/models/models.js
File app/models/models.js (right):

https://codereview.appspot.com/7201049/diff/1/app/models/models.js#newcode452
app/models/models.js:452: //console.log('Reset Application Database');
Just delete it IMO

https://codereview.appspot.com/7201049/diff/1/app/models/models.js#newcode460
app/models/models.js:460: //console.log('Delta');
Just delete it IMO

https://codereview.appspot.com/7201049/diff/1/app/store/env.js
File app/store/env.js (right):

https://codereview.appspot.com/7201049/diff/1/app/store/env.js#newcode32
app/store/env.js:32: //console.log('Env Init');
Just delete it IMO

https://codereview.appspot.com/7201049/diff/1/app/store/env.js#newcode56
app/store/env.js:56: //console.log('Env Connect');
Just delete it IMO, and so on for all other commented-out console.log
statements.

https://codereview.appspot.com/7201049/

« Back to merge proposal