yellow team mailing list archive

[Francesco Banconi <francesco.banconi@xxxxxxxxxxxxx>]

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.