Thanks for your review, guys!
https://codereview.appspot.com/6853044/diff/1/Makefile File Makefile (right): https://codereview.appspot.com/6853044/diff/1/Makefile#newcode21 Makefile:21: app/assets/stylesheets/all-static.css On 2012/11/13 17:08:24, matthew.scott wrote: > I think these are tabs (right angle-quotes in Rietveld). Should probably be > spaces to be safe. Yeap. They are tabs. I dont know if this is a Makefile rule, but in cases like this one above we should use tabs instead of spaces. https://codereview.appspot.com/6853044/diff/1/app/index.html File app/index.html (right): https://codereview.appspot.com/6853044/diff/1/app/index.html#newcode10 app/index.html:10: <link rel="stylesheet" href="/assets/stylesheets/all-static.css"> On 2012/11/13 17:27:44, francesco.banconi wrote: > Here the juju-ui prefix is missing. Done. https://codereview.appspot.com/6853044/diff/1/bin/merge-files File bin/merge-files (right): https://codereview.appspot.com/6853044/diff/1/bin/merge-files#newcode34 bin/merge-files:34: thirdPartyCss; On 2012/11/13 17:08:24, matthew.scott wrote: > Minor, and totally open to debate (anyone else, feel free to chime in) - I > prefer thirdPartyCSS, personally, because it's an acronym and you pronounce each > of the letters. Again, just a minor, though, Done. https://codereview.appspot.com/6853044/diff/1/lib/server.js File lib/server.js (right): https://codereview.appspot.com/6853044/diff/1/lib/server.js#newcode77 lib/server.js:77: server.get('/assets/stylesheets/:file', function(req, res) { On 2012/11/13 17:27:44, francesco.banconi wrote: > We discussed this on IRC: adding a 'juju-ui' prefix here and before (in > "/assets/all-third.js") allows us to easily integrate the gui in other projects. > As you said, express should still work after this change. Done. https://codereview.appspot.com/6853044/diff/1/lib/server.js#newcode88 lib/server.js:88: // "./app/assets/stylesheets" directory. On 2012/11/13 17:08:24, matthew.scott wrote: > I think this is a good way to do it, so long as the overhead isn't too great (I > expect not) This is lightning fast. https://codereview.appspot.com/6853044/ -- https://code.launchpad.net/~tveronezi/juju-gui/combinecss/+merge/134140 Your team Juju GUI Hackers is requested to review the proposed merge of lp:~tveronezi/juju-gui/combinecss 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
