Thiago I think the work you've done here is good. I'm a bit unclear how much still needs to be done to finish off this refactoring of the build structure.
Thanks for tackling it. The branch looks good to land after you make the changes suggested and weigh the other ones. https://codereview.appspot.com/6844061/diff/1/Makefile File Makefile (right): https://codereview.appspot.com/6844061/diff/1/Makefile#newcode18 Makefile:18: PRODUCTION_FILES=build/juju-ui/assets/modules.js \ I'd be tempted to make a symbol for 'build/juju-ui/assets' since it is repeated so often. https://codereview.appspot.com/6844061/diff/1/Makefile#newcode89 Makefile:89: @cd build && python -m SimpleHTTPServer 8888 I'm unhappy that the magic port 8888 is found four times in our code and twice in documentation. I wish it were possible to define it once, somewhere but am at a loss for how to do it. https://codereview.appspot.com/6844061/diff/1/Makefile#newcode94 Makefile:94: @rm -Rf build/ Yay, the clean target is much simplified. https://codereview.appspot.com/6844061/diff/1/Makefile#newcode100 Makefile:100: I'm concerned about separate build and install targets and having template.js in a separate rule from the others. Seems complicated and twisty. https://codereview.appspot.com/6844061/diff/1/app/index.html File app/index.html (right): https://codereview.appspot.com/6844061/diff/1/app/index.html#newcode14 app/index.html:14: <link rel="stylesheet" href="http://yui.yahooapis.com/combo?3.8.0pr1/build/slider-base/assets/skins/sam/slider-base.css";> Is this safe? The URL with an embedded version number looks fragile. https://codereview.appspot.com/6844061/diff/1/app/index.html#newcode15 app/index.html:15: <link> What is the second <link> for? https://codereview.appspot.com/6844061/diff/1/grunt.js File grunt.js (right): https://codereview.appspot.com/6844061/diff/1/grunt.js#newcode8 grunt.js:8: outputImage: 'stylesheets/sprite.png', I guess putting sprites in 'stylesheets' is a lie we can live with. https://codereview.appspot.com/6844061/ -- https://code.launchpad.net/~tveronezi/juju-gui/make-build/+merge/134706 Your team Juju GUI Hackers is requested to review the proposed merge of lp:~tveronezi/juju-gui/make-build into lp:juju-gui. -- Mailing list: https://launchpad.net/~yellow Post to : [email protected] Unsubscribe : https://launchpad.net/~yellow More help : https://help.launchpad.net/ListHelp
