← Back to team overview

yellow team mailing list archive

Re: Resource minification/aggregation css (issue 6853044)

 

Some minors and one question for everyone, but looks good to me (and
works well after the new push).  Thanks.


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
I think these are tabs (right angle-quotes in Rietveld).  Should
probably be spaces to be safe.

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;
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,

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#newcode88
lib/server.js:88: // "./app/assets/stylesheets" directory.
I think this is a good way to do it, so long as the overhead isn't too
great (I expect not)

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.


References