yellow team mailing list archive

[Francesco Banconi <francesco.banconi@xxxxxxxxxxxxx>]

Land with changes.

This review is done by me and Nicola.
This branch looks great Gary, it solves a lot of problems.
The only one issue we found is the presence of failures in test-debug,
but below we are suggesting a solution.
Thanks also for the documentation clean up (especially the how to make
releases part).
Nicola says: special thanks for renaming the build-devel target, you da
man! :-)



https://codereview.appspot.com/7005044/diff/1/Makefile
File Makefile (left):

https://codereview.appspot.com/7005044/diff/1/Makefile#oldcode455
Makefile:455: .PHONY: appcache-force appcache-touch beautify build \
Isn't build a phony target now?

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

https://codereview.appspot.com/7005044/diff/1/Makefile#newcode32
Makefile:32: -e '^app/assets/javascripts/gallery-.*\.js$$' \
Really nice, thank you.

https://codereview.appspot.com/7005044/diff/1/app/modules-debug.js
File app/modules-debug.js (right):

https://codereview.appspot.com/7005044/diff/1/app/modules-debug.js#newcode29
app/modules-debug.js:29: fullpath:
'juju-ui/assets/javascripts/gallery-timer-debug.js'
These three paths need to be absolute (add a slash at the beginning of
each one). Otherwise, in test-debug runs, they are searched inside the
/test/ path, and a bunch of tests fail.

https://codereview.appspot.com/7005044/diff/1/app/views/utils.js
File app/views/utils.js (right):

https://codereview.appspot.com/7005044/diff/1/app/views/utils.js#newcode121
app/views/utils.js:121: debug: noop
Yay! No more errors running test-prod!

https://codereview.appspot.com/7005044/

-- 
https://code.launchpad.net/~gary/juju-gui/grabbag/+merge/141022
Your team Juju GUI Hackers is requested to review the proposed merge of lp:~gary/juju-gui/grabbag into lp:juju-gui.