Looks good and the latest revision fixes the GlobalConfig problem. Thank you Thiago. Some comments follow.
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"> Here the juju-ui prefix is missing. 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, FWIW, I prefer CSS too. 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) { 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. 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
