yellow team mailing list archive

[Thiago Veronezi <thiago@xxxxxxxxxxxx>]

Thanks, Benji!


https://codereview.appspot.com/6821090/diff/7001/Makefile
File Makefile (right):

https://codereview.appspot.com/6821090/diff/7001/Makefile#newcode12
Makefile:12: node_modules/grunt node_modules/node-spritesheet
node_modules/node-minify
This line does not have more than 80 chars, but I changed it anyway.

On 2012/11/07 17:05:18, benji wrote:
> This line is too long.

https://codereview.appspot.com/6821090/diff/7001/Makefile#newcode75
Makefile:75: @cp app/assets/combine/properties-dev.js
app/assets/javascripts/generated/properties.js
On 2012/11/07 17:05:18, benji wrote:
> This line is too long.

Done.

https://codereview.appspot.com/6821090/diff/7001/Makefile#newcode80
Makefile:80: @cp app/assets/combine/properties-prod.js
app/assets/javascripts/generated/properties.js
On 2012/11/07 17:05:18, benji wrote:
> This line is too long.

Done.

https://codereview.appspot.com/6821090/diff/7001/app/assets/combine/merge-files.js
File app/assets/combine/merge-files.js (right):

https://codereview.appspot.com/6821090/diff/7001/app/assets/combine/merge-files.js#newcode1
app/assets/combine/merge-files.js:1: 'use strict';
This is entirely ours.

On 2012/11/07 17:05:18, benji wrote:
> Is this code we wrote or was it imported from somewhere?  If it was
written
> elsewhere, we need to specify its license and it would be nice to know
where it
> came from.

> I will forgo reviewing the code until I know the answers to the above.

https://codereview.appspot.com/6821090/diff/7001/app/index.html
File app/index.html (right):

https://codereview.appspot.com/6821090/diff/7001/app/index.html#newcode72
app/index.html:72: <script src="/source/yui.js"></script>
What about '/assets/yui.js'? Id this ok?

On 2012/11/07 17:05:18, benji wrote:
> We have been calling these things "assets".  If the change to "source"
was a
> considered one, then I am fine with it.  If not, we should continue to
use the
> same terminology that we have been using.

https://codereview.appspot.com/6821090/diff/7001/lib/server.js
File lib/server.js (right):

https://codereview.appspot.com/6821090/diff/7001/lib/server.js#newcode50
lib/server.js:50: server.get('/source/all-third.js', function(req, res)
{
I want to hide the real path.
I can change it to '/assets/all-third.js', would it be ok?

On 2012/11/07 17:05:18, benji wrote:
> Why should the path exposed via HTTP ("source") be different than the
path on
> disk ("assets")?

https://codereview.appspot.com/6821090/

-- 
https://code.launchpad.net/~tveronezi/juju-gui/uglify/+merge/133116
Your team Juju GUI Hackers is requested to review the proposed merge of lp:~tveronezi/juju-gui/uglify into lp:juju-gui.