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#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.
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 FILES=build/ juju-ui/ assets/ modules. js \ juju-ui/ assets' since it is
Makefile:18: PRODUCTION_
I'd be tempted to make a symbol for 'build/
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 yui.yahooapis. com/combo? 3.8.0pr1/ build/slider- base/assets/ skins/sam/ slider- base.css">
app/index.html:14: <link rel="stylesheet"
href="http://
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 sprite. png',
grunt.js:8: outputImage: 'stylesheets/
I guess putting sprites in 'stylesheets' is a lie we can live with.
https:/ /codereview. appspot. com/6844061/