juju-gui

Merge lp:~tveronezi/juju-gui/make-build into lp:juju-gui/experimental

  1. make-build
  2. Merge into trunk
Proposed by Thiago Veronezi
Status: Merged
Approved by: Gary Poster
Approved revision: 257
Merged at revision: 251
Proposed branch: lp:~tveronezi/juju-gui/make-build
Merge into: lp:juju-gui/experimental
Diff against target: 438 lines (+83/-146)
10 files modified
.bzrignore (+1/-9)
Makefile (+33/-31)
app/index.html (+3/-3)
app/views/environment.js (+1/-0)
bin/merge-files (+20/-36)
grunt.js (+3/-3)
lib/server.js (+15/-60)
lib/templates.js (+2/-2)
test-server.js (+4/-1)
test/index.html (+1/-1)
To merge this branch: bzr merge lp:~tveronezi/juju-gui/make-build
Reviewer Review Type Date Requested Status
Juju GUI Hackers Pending
Review via email: mp+134706@code.launchpad.net

Description of the change

Makefile does not support static file deployment

Full description here: https://bugs.launchpad.net/juju-gui/+bug/1078898

This branch does not support CSS minification. It only combines the third-party css files. The minification of the YUI and our custom CSS files is covered by another branch;

The "make debug" command starts the node.js server with the debug version of our application;

The "make server" command starts a python SimpleHTTPServer service with the production version of our application. Note that it is not possible to use REST with this version. It will be covered by another card;

https://codereview.appspot.com/6844061/

To post a comment you must log in.
Revision history for this message
Thiago Veronezi (tveronezi) wrote :

Reviewers: mp+134706_code.launchpad.net,

Message:
Please take a look.

Description:
Makefile does not support static file deployment

Full description here: https://bugs.launchpad.net/juju-gui/+bug/1078898

This branch does not support CSS minification. It only combines the
third-party css files. The minification of the YUI and our custom CSS
files is covered by another branch;

The "make debug" command starts the node.js server with the debug
version of our application;

The "make server" command starts a python SimpleHTTPServer service with
the production version of our application. Note that it is not possible
to use REST with this version. It will be covered by another card;

https://code.launchpad.net/~tveronezi/juju-gui/make-build/+merge/134706

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/6844061/

Affected files:
   M .bzrignore
   M Makefile
   A [revision details]
   M app/index.html
   M bin/merge-files
   M grunt.js
   M lib/server.js
   M lib/templates.js

lp:~tveronezi/juju-gui/make-build updated
252. By Thiago Veronezi

personal code review

Revision history for this message
Thiago Veronezi (tveronezi) wrote :

https://codereview.appspot.com/6844061/diff/1/bin/merge-files
File bin/merge-files (right):

https://codereview.appspot.com/6844061/diff/1/bin/merge-files#newcode63
bin/merge-files:63: filesToLoad.js.push.apply(filesToLoad.js, [
'./app/assets/javascripts/d3.v2.min.js',
Please, ignore this long line. Its is already fixed.

https://codereview.appspot.com/6844061/

Revision history for this message
Brad Crittenden (bac) wrote :

Thiago I think the work you've done here is good. I'm a bit unclear how
much still needs to be done to finish off this refactoring of the build
structure.

Thanks for tackling it. The branch looks good to land after you make
the changes suggested and weigh the other ones.

https://codereview.appspot.com/6844061/diff/1/Makefile
File Makefile (right):

https://codereview.appspot.com/6844061/diff/1/Makefile#newcode18
Makefile:18: PRODUCTION_FILES=build/juju-ui/assets/modules.js \
I'd be tempted to make a symbol for 'build/juju-ui/assets' since it is
repeated so often.

https://codereview.appspot.com/6844061/diff/1/Makefile#newcode89
Makefile:89: @cd build && python -m SimpleHTTPServer 8888
I'm unhappy that the magic port 8888 is found four times in our code and
twice in documentation. I wish it were possible to define it once,
somewhere but am at a loss for how to do it.

https://codereview.appspot.com/6844061/diff/1/Makefile#newcode94
Makefile:94: @rm -Rf build/
Yay, the clean target is much simplified.

https://codereview.appspot.com/6844061/diff/1/Makefile#newcode100
Makefile:100:
I'm concerned about separate build and install targets and having
template.js in a separate rule from the others. Seems complicated and
twisty.

https://codereview.appspot.com/6844061/diff/1/app/index.html
File app/index.html (right):

https://codereview.appspot.com/6844061/diff/1/app/index.html#newcode14
app/index.html:14: <link rel="stylesheet"
href="http://yui.yahooapis.com/combo?3.8.0pr1/build/slider-base/assets/skins/sam/slider-base.css">
Is this safe? The URL with an embedded version number looks fragile.

https://codereview.appspot.com/6844061/diff/1/app/index.html#newcode15
app/index.html:15: <link>
What is the second <link> for?

https://codereview.appspot.com/6844061/diff/1/grunt.js
File grunt.js (right):

https://codereview.appspot.com/6844061/diff/1/grunt.js#newcode8
grunt.js:8: outputImage: 'stylesheets/sprite.png',
I guess putting sprites in 'stylesheets' is a lie we can live with.

https://codereview.appspot.com/6844061/

lp:~tveronezi/juju-gui/make-build updated
253. By Thiago Veronezi

code review

Revision history for this message
Thiago Veronezi (tveronezi) wrote :

Thanks for your review Brad!

https://codereview.appspot.com/6844061/diff/1/Makefile
File Makefile (right):

https://codereview.appspot.com/6844061/diff/1/Makefile#newcode18
Makefile:18: PRODUCTION_FILES=build/juju-ui/assets/modules.js \
On 2012/11/19 18:18:58, bac wrote:
> I'd be tempted to make a symbol for 'build/juju-ui/assets' since it is
 repeated
> so often.

Done.

https://codereview.appspot.com/6844061/diff/1/Makefile#newcode89
Makefile:89: @cd build && python -m SimpleHTTPServer 8888
On 2012/11/19 18:18:58, bac wrote:
> I'm unhappy that the magic port 8888 is found four times in our code
 and twice
> in documentation. I wish it were possible to define it once,
 somewhere but am
> at a loss for how to do it.

I agree. I would like to create another card under the "Needs
specification" group for it, so we can discuss this issue later. Is that
ok?

https://codereview.appspot.com/6844061/diff/1/Makefile#newcode94
Makefile:94: @rm -Rf build/
On 2012/11/19 18:18:58, bac wrote:
> Yay, the clean target is much simplified.

:O)

https://codereview.appspot.com/6844061/diff/1/Makefile#newcode100
Makefile:100:
On 2012/11/19 18:18:58, bac wrote:
> I'm concerned about separate build and install targets and having
 template.js in
> a separate rule from the others. Seems complicated and twisty.

True. "install" does not mean "install". IMO "install" should be renamed
to "generate-files", or something like that. wdyt?

template.js should be in a separated rule, so we can determine when we
should execute it, right? The same applies to the sprites.

https://codereview.appspot.com/6844061/diff/1/app/index.html
File app/index.html (right):

https://codereview.appspot.com/6844061/diff/1/app/index.html#newcode14
app/index.html:14: <link rel="stylesheet"
href="http://yui.yahooapis.com/combo?3.8.0pr1/build/slider-base/assets/skins/sam/slider-base.css">
On 2012/11/19 18:18:58, bac wrote:
> Is this safe? The URL with an embedded version number looks fragile.

True. I've investigated it a little bit further, and it works if I add
the 'slider-base' requirement to the environment.js file. Tkx Brad!

https://codereview.appspot.com/6844061/diff/1/app/index.html#newcode15
app/index.html:15: <link>
On 2012/11/19 18:18:58, bac wrote:
> What is the second <link> for?
 for nothing :)
Removed tkx.

https://codereview.appspot.com/6844061/diff/1/grunt.js
File grunt.js (right):

https://codereview.appspot.com/6844061/diff/1/grunt.js#newcode8
grunt.js:8: outputImage: 'stylesheets/sprite.png',
On 2012/11/19 18:18:58, bac wrote:
> I guess putting sprites in 'stylesheets' is a lie we can live with.

I agree.

https://codereview.appspot.com/6844061/

Revision history for this message
Gary Poster (gary) wrote :

Thanks Thiago. This is a nice improvement. Once we have the CSS
changes, and we no longer have the slow initial combo CSS load from YUI,
this will be really nice to use. I have some small suggestions for the
Makefile, in line with Brad's comments.

https://codereview.appspot.com/6844061/diff/1/.bzrignore
File .bzrignore (right):

https://codereview.appspot.com/6844061/diff/1/.bzrignore#newcode16
.bzrignore:16: build
Nice simplification and clean-up.

https://codereview.appspot.com/6844061/diff/1/Makefile
File Makefile (right):

https://codereview.appspot.com/6844061/diff/1/Makefile#newcode100
Makefile:100:
On 2012/11/19 18:18:58, bac wrote:
> I'm concerned about separate build and install targets and having
 template.js in
> a separate rule from the others. Seems complicated and twisty.

Having "build" depend on "install" also seems odd, conceptually.

Does the "install" target bring any value? That is, is there any use
case for running it alone? If not, I suggest combining it and "build"
into the "build" target (and deleting the "install" target). What do
you two think of that?

https://codereview.appspot.com/6844061/diff/1/lib/server.js
File lib/server.js (right):

https://codereview.appspot.com/6844061/diff/1/lib/server.js#newcode57
lib/server.js:57: res.sendfile('build/juju-ui/assets/' + fileName);
Interesting. I wanted the development server code to be even simpler
than this, but I can see why what you did is compelling. I would be
tempted to experiment with converting the js files to symlinks in a
later branch and simplifying this code. This is an improvement anyway,
though, and maybe my hunch won't work out.

https://codereview.appspot.com/6844061/

lp:~tveronezi/juju-gui/make-build updated
254. By Thiago Veronezi

code review

255. By Thiago Veronezi

trunk merge

256. By Thiago Veronezi

code review

257. By Thiago Veronezi

code review

Revision history for this message
Thiago Veronezi (tveronezi) wrote :

Thanks Gary for your review!

https://codereview.appspot.com/6844061/diff/1/.bzrignore
File .bzrignore (right):

https://codereview.appspot.com/6844061/diff/1/.bzrignore#newcode16
.bzrignore:16: build
On 2012/11/19 19:00:33, gary.poster wrote:
> Nice simplification and clean-up.

Ohhh yeah. :O)

https://codereview.appspot.com/6844061/diff/1/Makefile
File Makefile (right):

https://codereview.appspot.com/6844061/diff/1/Makefile#newcode100
Makefile:100:
On 2012/11/19 19:00:33, gary.poster wrote:
> On 2012/11/19 18:18:58, bac wrote:
> > I'm concerned about separate build and install targets and having
 template.js
> in
> > a separate rule from the others. Seems complicated and twisty.

> Having "build" depend on "install" also seems odd, conceptually.

> Does the "install" target bring any value? That is, is there any use
case for
> running it alone? If not, I suggest combining it and "build" into the
"build"
> target (and deleting the "install" target). What do you two think of
that?

Ah... I see! I will delete the "install" target and change the "build"
target from...

build: install

... to...

build: appcache $(NODE_TARGETS) build/juju-ui/templates.js yuidoc
spritegen combinejs

Is that ok?

https://codereview.appspot.com/6844061/diff/1/lib/server.js
File lib/server.js (right):

https://codereview.appspot.com/6844061/diff/1/lib/server.js#newcode57
lib/server.js:57: res.sendfile('build/juju-ui/assets/' + fileName);
On 2012/11/19 19:00:33, gary.poster wrote:
> Interesting. I wanted the development server code to be even simpler
 than this,
> but I can see why what you did is compelling. I would be tempted to
 experiment
> with converting the js files to symlinks in a later branch and
 simplifying this
> code. This is an improvement anyway, though, and maybe my hunch won't
 work out.

I needed to change the "test-server.js" file too. The tests were broken
due to the new notification stuff. I've tried to use the symbolic link
approach in order to avoid it, but with no success.

I will use the "new branch" option to do it. Thanks.

https://codereview.appspot.com/6844061/

Revision history for this message
Thiago Veronezi (tveronezi) wrote :

Please take a look.

https://codereview.appspot.com/6844061/

Revision history for this message
Gary Poster (gary) wrote :

Good, Thiago, thank you. The slow CSS loading is the only downside for
putting this branch in staging, and if you can follow it quickly with
the CSS branch then I think it will be great.

+1!

Gary

https://codereview.appspot.com/6844061/diff/1/Makefile
File Makefile (right):

https://codereview.appspot.com/6844061/diff/1/Makefile#newcode89
Makefile:89: @cd build && python -m SimpleHTTPServer 8888
On 2012/11/19 18:50:59, thiago wrote:
> On 2012/11/19 18:18:58, bac wrote:
> > I'm unhappy that the magic port 8888 is found four times in our code
 and twice
> > in documentation. I wish it were possible to define it once,
 somewhere but am
> > at a loss for how to do it.

> I agree. I would like to create another card under the "Needs
specification"
> group for it, so we can discuss this issue later. Is that ok?

If you have any ideas on what we can do, +1. Otherwise, eh, don't
bother IMO.

https://codereview.appspot.com/6844061/diff/1/lib/server.js
File lib/server.js (right):

https://codereview.appspot.com/6844061/diff/1/lib/server.js#newcode57
lib/server.js:57: res.sendfile('build/juju-ui/assets/' + fileName);
On 2012/11/19 20:23:19, thiago wrote:
> On 2012/11/19 19:00:33, gary.poster wrote:
> > Interesting. I wanted the development server code to be even
 simpler than
> this,
> > but I can see why what you did is compelling. I would be tempted to
> experiment
> > with converting the js files to symlinks in a later branch and
 simplifying
> this
> > code. This is an improvement anyway, though, and maybe my hunch
 won't work
> out.

> I needed to change the "test-server.js" file too. The tests were
broken due to
> the new notification stuff. I've tried to use the symbolic link
approach in
> order to avoid it, but with no success.

> I will use the "new branch" option to do it. Thanks.

Cool. I'm not sure it is that worth it to mess with it, given that you
have already experimented with it. That said, bug 1078910 will lead to
a natural opportunity to investigate.

https://codereview.appspot.com/6844061/diff/6002/Makefile
File Makefile (right):

https://codereview.appspot.com/6844061/diff/6002/Makefile#newcode16
Makefile:16: BUILD_ASSETS_DIR=build/juju-ui/assets
This ends up saving two characters per reference. :-) I guess it is
still an advantage to prevent typos though. Well...not even much there,
since bash doesn't have errors for unknown names. I dunno, I'm
certainly fine with this, but it didn't end up helping much IMO. Maybe
bac has a different suggestion for a name?

https://codereview.appspot.com/6844061/diff/6002/Makefile#newcode95
Makefile:95: build: appcache $(NODE_TARGETS) build/juju-ui/templates.js
yuidoc spritegen combinejs
I think "build" is the right name logically. benji might not like the
fact that we are breaking backwards compatibility for the dev
experience, but since the "all" target is equivalent and still works, I
think it is fine.

https://codereview.appspot.com/6844061/

Revision history for this message
Thiago Veronezi (tveronezi) wrote :

*** Submitted:

Makefile does not support static file deployment

Full description here: https://bugs.launchpad.net/juju-gui/+bug/1078898

This branch does not support CSS minification. It only combines the
third-party css files. The minification of the YUI and our custom CSS
files is covered by another branch;

The "make debug" command starts the node.js server with the debug
version of our application;

The "make server" command starts a python SimpleHTTPServer service with
the production version of our application. Note that it is not possible
to use REST with this version. It will be covered by another card;

R=bac, gary.poster
CC=
https://codereview.appspot.com/6844061

https://codereview.appspot.com/6844061/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file '.bzrignore'
--- .bzrignore 2012-11-13 15:04:19 +0000
+++ .bzrignore 2012-11-19 20:22:19 +0000
@@ -3,22 +3,14 @@
3gui.sublime-project3gui.sublime-project
4.emacs.desktop.lock4.emacs.desktop.lock
5gui.sublime-workspace5gui.sublime-workspace
6app/templates.js
7app/assets/stylesheets/juju-gui.css
8app/assets/javascripts/d3.v2.js6app/assets/javascripts/d3.v2.js
9app/assets/javascripts/d3.v2.min.js7app/assets/javascripts/d3.v2.min.js
10app/assets/javascripts/yui8app/assets/javascripts/yui
11app/templates.js
12app/assets/stylesheets/juju-gui.css
13docs/_build/doctrees9docs/_build/doctrees
14docs/_build/html10docs/_build/html
15*~11*~
16app/assets/stylesheets/juju-gui.css
17tags12tags
18Session.vim13Session.vim
19virtualenv14virtualenv
20yuidoc15yuidoc
21app/assets/manifest.appcache16build
22app/assets/sprite
23app/assets/javascripts/generated/
24app/assets/stylesheets/all-static.css
2517
=== modified file 'Makefile'
--- Makefile 2012-11-15 13:07:29 +0000
+++ Makefile 2012-11-19 20:22:19 +0000
@@ -13,18 +13,20 @@
13 node_modules/node-minify13 node_modules/node-minify
14TEMPLATE_TARGETS=$(shell bzr ls -k file app/templates)14TEMPLATE_TARGETS=$(shell bzr ls -k file app/templates)
15SPRITE_SOURCE_FILES=$(shell bzr ls -R -k file app/assets/images)15SPRITE_SOURCE_FILES=$(shell bzr ls -R -k file app/assets/images)
16SPRITE_GENERATED_FILES=app/assets/sprite/sprite.css app/assets/sprite/sprite.png16BUILD_ASSETS_DIR=build/juju-ui/assets
17COMPRESSED_FILES=app/assets/javascripts/generated/all-app-debug.js \17SPRITE_GENERATED_FILES=$(BUILD_ASSETS_DIR)/stylesheets/sprite.css \
18 app/assets/javascripts/generated/all-app.js \18 $(BUILD_ASSETS_DIR)/stylesheets/sprite.png
19 app/assets/javascripts/generated/all-third.js \19PRODUCTION_FILES=$(BUILD_ASSETS_DIR)/modules.js \
20 app/assets/javascripts/generated/all-yui.js \20 $(BUILD_ASSETS_DIR)/config.js \
21 app/assets/stylesheets/all-static.css21 $(BUILD_ASSETS_DIR)/app.js \
22 $(BUILD_ASSETS_DIR)/stylesheets/all-static.css
22DATE=$(shell date -u)23DATE=$(shell date -u)
23APPCACHE=app/assets/manifest.appcache24APPCACHE=$(BUILD_ASSETS_DIR)/manifest.appcache
2425
25all: install26all: build
2627
27app/templates.js: $(TEMPLATE_TARGETS) bin/generateTemplates28build/juju-ui/templates.js: $(TEMPLATE_TARGETS) bin/generateTemplates
29 @test -d "$(BUILD_ASSETS_DIR)/stylesheets" || mkdir -p "$(BUILD_ASSETS_DIR)/stylesheets"
28 @./bin/generateTemplates30 @./bin/generateTemplates
2931
30yuidoc/index.html: node_modules/yuidocjs $(JSFILES)32yuidoc/index.html: node_modules/yuidocjs $(JSFILES)
@@ -34,8 +36,6 @@
3436
35$(SPRITE_GENERATED_FILES): node_modules/grunt node_modules/node-spritesheet $(SPRITE_SOURCE_FILES)37$(SPRITE_GENERATED_FILES): node_modules/grunt node_modules/node-spritesheet $(SPRITE_SOURCE_FILES)
36 @node_modules/grunt/bin/grunt spritegen38 @node_modules/grunt/bin/grunt spritegen
37 @rm -Rf app/assets/sprite/
38 @mv bin/sprite app/assets
3939
40$(NODE_TARGETS): package.json40$(NODE_TARGETS): package.json
41 @npm install41 @npm install
@@ -43,8 +43,6 @@
43 @ln -sf `pwd`/node_modules/yui ./app/assets/javascripts/43 @ln -sf `pwd`/node_modules/yui ./app/assets/javascripts/
44 @ln -sf `pwd`/node_modules/d3/d3.v2* ./app/assets/javascripts/44 @ln -sf `pwd`/node_modules/d3/d3.v2* ./app/assets/javascripts/
4545
46install: appcache $(NODE_TARGETS) app/templates.js yuidoc spritegen combinejs
47
48gjslint: virtualenv/bin/gjslint46gjslint: virtualenv/bin/gjslint
49 @virtualenv/bin/gjslint --strict --nojsdoc --jslint_error=all \47 @virtualenv/bin/gjslint --strict --nojsdoc --jslint_error=all \
50 --custom_jsdoc_tags module,main,class,method,event,property,attribute,submodule,namespace,extends,config,constructor,static,final,readOnly,writeOnce,optional,required,param,return,for,type,private,protected,requires,default,uses,example,chainable,deprecated,since,async,beta,bubbles,extension,extensionfor,extension_for \48 --custom_jsdoc_tags module,main,class,method,event,property,attribute,submodule,namespace,extends,config,constructor,static,final,readOnly,writeOnce,optional,required,param,return,for,type,private,protected,requires,default,uses,example,chainable,deprecated,since,async,beta,bubbles,extension,extensionfor,extension_for \
@@ -67,36 +65,40 @@
6765
68spritegen: $(SPRITE_GENERATED_FILES)66spritegen: $(SPRITE_GENERATED_FILES)
6967
70$(COMPRESSED_FILES): node_modules/yui node_modules/d3/d3.v2.min.js $(JSFILES) ./bin/merge-files68$(PRODUCTION_FILES): node_modules/yui node_modules/d3/d3.v2.min.js $(JSFILES) ./bin/merge-files
71 @rm -f app/assets/stylesheets/all-static.css69 @rm -f $(PRODUCTION_FILES)
72 @rm -Rf app/assets/javascripts/generated/70 @test -d "$(BUILD_ASSETS_DIR)/stylesheets" || mkdir -p "$(BUILD_ASSETS_DIR)/stylesheets"
73 @mkdir app/assets/javascripts/generated/
74 @./bin/merge-files71 @./bin/merge-files
72 @cp app/modules.js $(BUILD_ASSETS_DIR)/modules.js
73 @cp app/config.js $(BUILD_ASSETS_DIR)/config.js
7574
76combinejs: $(COMPRESSED_FILES)75combinejs: $(PRODUCTION_FILES)
7776
78prep: beautify lint77prep: beautify lint
7978
80test: install79test: build
81 @./test-server.sh80 @./test-server.sh
8281
83debug: install82debug: build
84 @echo "Customize config.js to modify server settings"
85 @node server.js debug
86
87server: install
88 @echo "Customize config.js to modify server settings"83 @echo "Customize config.js to modify server settings"
89 @node server.js84 @node server.js
9085
86server: build
87 @echo "Runnning the application from a SimpleHTTPServer"
88 @cd build && python -m SimpleHTTPServer 8888
89
91clean:90clean:
92 @rm -rf node_modules virtualenv91 @rm -rf node_modules virtualenv
93 @make -C docs clean92 @make -C docs clean
94 @rm -Rf bin/sprite/93 @rm -Rf build/
95 @rm -Rf app/assets/sprite/94
96 @rm -Rf app/assets/javascripts/generated/95build: appcache $(NODE_TARGETS) build/juju-ui/templates.js yuidoc spritegen combinejs
97 @rm -f app/assets/stylesheets/all-static.css96 @cp -f app/index.html build/
97 @cp -rf app/assets/images $(BUILD_ASSETS_DIR)/images
98 @cp -rf app/assets/svgs $(BUILD_ASSETS_DIR)/svgs
9899
99$(APPCACHE): manifest.appcache.in100$(APPCACHE): manifest.appcache.in
101 @test -d "build/juju-ui/assets" || mkdir -p "build/juju-ui/assets"
100 @cp manifest.appcache.in $(APPCACHE)102 @cp manifest.appcache.in $(APPCACHE)
101 @sed -re 's/^\# TIMESTAMP .+$$/\# TIMESTAMP $(DATE)/' -i $(APPCACHE)103 @sed -re 's/^\# TIMESTAMP .+$$/\# TIMESTAMP $(DATE)/' -i $(APPCACHE)
102104
@@ -110,6 +112,6 @@
110# appcache, and this provides the correct order.112# appcache, and this provides the correct order.
111appcache-force: appcache-touch appcache113appcache-force: appcache-touch appcache
112114
113.PHONY: test lint beautify server install clean prep jshint gjslint \115.PHONY: test lint beautify server build clean prep jshint gjslint \
114 appcache appcache-touch appcache-force yuidoc spritegen yuidoc-lint \116 appcache appcache-touch appcache-force yuidoc spritegen yuidoc-lint \
115 combinejs117 combinejs
116118
=== modified file 'app/index.html'
--- app/index.html 2012-11-15 15:44:00 +0000
+++ app/index.html 2012-11-19 20:22:19 +0000
@@ -9,7 +9,7 @@
9 <link href='http://fonts.googleapis.com/css?family=Ubuntu:400,400italic&subset=latin,latin-ext' rel='stylesheet' type='text/css'>9 <link href='http://fonts.googleapis.com/css?family=Ubuntu:400,400italic&subset=latin,latin-ext' rel='stylesheet' type='text/css'>
10 <link rel="stylesheet" href="/juju-ui/assets/stylesheets/all-static.css">10 <link rel="stylesheet" href="/juju-ui/assets/stylesheets/all-static.css">
11 <link rel="stylesheet" href="/juju-ui/assets/stylesheets/juju-gui.css">11 <link rel="stylesheet" href="/juju-ui/assets/stylesheets/juju-gui.css">
12 <link rel="stylesheet" href="/juju-ui/assets/sprite/sprite.css">12 <link rel="stylesheet" href="/juju-ui/assets/stylesheets/sprite.css">
13 <!--[if lt IE 9]>13 <!--[if lt IE 9]>
14 <script src="http://html5shim.googlecode.com/svn/trunk/html5.js"></script>14 <script src="http://html5shim.googlecode.com/svn/trunk/html5.js"></script>
15 <![endif]-->15 <![endif]-->
@@ -68,9 +68,9 @@
68 <div id="vp-right-border"></div>68 <div id="vp-right-border"></div>
69 </div>69 </div>
70 <!-- javascript away -->70 <!-- javascript away -->
71 <script src="/juju-ui/assets/yui.js"></script>71 <script src="/juju-ui/assets/app.js"></script>
72 <script src="/juju-ui/assets/all-third.js"></script>
73 <script src="/juju-ui/assets/modules.js"></script>72 <script src="/juju-ui/assets/modules.js"></script>
73 <script src="/juju-ui/assets/config.js"></script>
74 <script>74 <script>
75 YUI(GlobalConfig).use(["juju-gui"], function(Y) {75 YUI(GlobalConfig).use(["juju-gui"], function(Y) {
76 app = new Y.juju.App(juju_config);76 app = new Y.juju.App(juju_config);
7777
=== modified file 'app/views/environment.js'
--- app/views/environment.js 2012-11-19 16:20:21 +0000
+++ app/views/environment.js 2012-11-19 20:22:19 +0000
@@ -1814,5 +1814,6 @@
1814 'svg-layouts',1814 'svg-layouts',
1815 'event-resize',1815 'event-resize',
1816 'slider',1816 'slider',
1817 'slider-base',
1817 'view']1818 'view']
1818});1819});
18191820
=== modified file 'bin/merge-files'
--- bin/merge-files 2012-11-15 13:07:29 +0000
+++ bin/merge-files 2012-11-19 20:22:19 +0000
@@ -30,8 +30,7 @@
30 syspath = require('path'),30 syspath = require('path'),
31 paths,31 paths,
32 reqs,32 reqs,
33 filesToLoad,33 filesToLoad;
34 thirdPartyCSS;
3534
36 // First we find all of the paths to our custom Javascript in the app35 // First we find all of the paths to our custom Javascript in the app
37 // directory. We need to tell the function to ignore the "assets" directory36 // directory. We need to tell the function to ignore the "assets" directory
@@ -42,6 +41,9 @@
42 [syspath.join(process.cwd(), './app/assets'),41 [syspath.join(process.cwd(), './app/assets'),
43 syspath.join(process.cwd(), './app/modules-debug.js')]);42 syspath.join(process.cwd(), './app/modules-debug.js')]);
4443
44 // templates.js is a generated file. It is not part of the app directory.
45 paths.push(syspath.join(process.cwd(), './build/juju-ui/templates.js'));
46
45 // Get the name of all the YUI modules that our custom js files use directly.47 // Get the name of all the YUI modules that our custom js files use directly.
46 reqs = merge.loadRequires(paths);48 reqs = merge.loadRequires(paths);
4749
@@ -56,41 +58,23 @@
5658
57 // Get all of the YUI files and their dependencies59 // Get all of the YUI files and their dependencies
58 filesToLoad = merge.getYUIFiles(reqs);60 filesToLoad = merge.getYUIFiles(reqs);
59 thirdPartyCSS = filesToLoad.css;61
60 thirdPartyCSS.push('./app/assets/stylesheets/bootstrap-2.0.4.css');62 // Merge third-party files to the filesToLoad list
61 thirdPartyCSS.push('./app/assets/stylesheets/bootstrap-responsive-2.0.4.css');63 filesToLoad.js.push.apply(filesToLoad.js, [
62 console.log('CSS: ' + thirdPartyCSS);64 './app/assets/javascripts/d3.v2.min.js',
6365 './app/assets/javascripts/d3-components.js',
64 // Combine YUI js files66 './app/assets/javascripts/reconnecting-websocket.js',
67 './app/assets/javascripts/svg-layouts.js' ]);
68
69 // Merge our list of custom js files to the filesToLoad list
70 filesToLoad.js.push.apply(filesToLoad.js, paths);
71
72 // Combine YUI and our js files
65 merge.combine(filesToLoad.js,73 merge.combine(filesToLoad.js,
66 './app/assets/javascripts/generated/all-yui.js', 'uglifyjs');74 './build/juju-ui/assets/app.js', 'uglifyjs');
6775
68 // Combine third party css files76 // Combine third party css files
69 merge.combine(thirdPartyCSS,77 merge.combine([ './app/assets/stylesheets/bootstrap-2.0.4.css',
70 './app/assets/stylesheets/all-static.css', 'yui-css');78 './app/assets/stylesheets/bootstrap-responsive-2.0.4.css' ],
7179 './build/juju-ui/assets/stylesheets/all-static.css', 'yui-css');
72 // Combine third party js libraries
73 merge.combine([ './app/assets/javascripts/d3.v2.min.js',
74 './app/assets/javascripts/d3-components.js',
75 './app/assets/javascripts/reconnecting-websocket.js',
76 './app/assets/javascripts/svg-layouts.js' ],
77 './app/assets/javascripts/generated/all-third-min.js', 'uglifyjs');
78
79 // It has only one file but eventually it will have more.
80 merge.combine([ './app/assets/javascripts/d3.v2.js' ],
81 './app/assets/javascripts/generated/all-third.js', 'uglifyjs');
82
83 // Now we only need to generate the file that is used to tell YUI where all
84 // the dependencies are. We either use a debug version or the production
85 // version. The debug version is simply pointers to the individual files.
86 // The production version aggregates all of the files together.
87
88 // Creating the combined file for the modules-debug.js and config.js files
89 merge.combine(['./app/modules-debug.js', './app/config.js'],
90 './app/assets/javascripts/generated/all-app-debug.js');
91
92 // Creating the combined file for all our files. Note that this includes
93 // app/modules.js in the rollup, which is why that file still needs exist.
94 merge.combine(paths, './app/assets/javascripts/generated/all-app.js',
95 'uglifyjs');
96});80});
9781
=== modified file 'grunt.js'
--- grunt.js 2012-11-06 12:28:51 +0000
+++ grunt.js 2012-11-19 20:22:19 +0000
@@ -5,13 +5,13 @@
5 spritesheet: {5 spritesheet: {
6 compile: {6 compile: {
7 options: {7 options: {
8 outputImage: 'sprite/sprite.png',8 outputImage: 'stylesheets/sprite.png',
9 outputCss: 'sprite/sprite.css',9 outputCss: 'stylesheets/sprite.css',
10 selector: '.sprite'10 selector: '.sprite'
1111
12 },12 },
13 files: {13 files: {
14 'bin': 'app/assets/images/*'14 'build/juju-ui/assets': 'app/assets/images/*'
15 }15 }
16 }16 }
17 }17 }
1818
=== modified file 'lib/server.js'
--- lib/server.js 2012-11-15 13:07:29 +0000
+++ lib/server.js 2012-11-19 20:22:19 +0000
@@ -5,35 +5,10 @@
5 fs = require('fs'),5 fs = require('fs'),
6 path = require('path'),6 path = require('path'),
7 config = require('../config').config.server,7 config = require('../config').config.server,
8 debugMode = (String(process.argv[2]).toLowerCase() === 'debug'),
9 public_dir = config.public_dir,8 public_dir = config.public_dir,
10 Templates = require('./templates.js'),9 Templates = require('./templates.js'),
11 view = require('./view.js');10 view = require('./view.js');
1211
13
14/**
15 * It finds a file under the given path. The first match wins.
16 * @param {string} path starting point.
17 * @param {string} fileName name of the file to be found.
18 * @return {string} the file path.
19 */
20function findFile(path, fileName) {
21 var file, files = fs.readdirSync(path);
22 for (var i = 0; i < files.length; i = i + 1) {
23 if (files[i] === fileName) {
24 return path + '/' + files[i];
25 }
26 file = fs.statSync(path + '/' + files[i]);
27 if (file.isDirectory()) {
28 file = findFile(path + '/' + files[i], fileName);
29 if (file) {
30 return file;
31 }
32 }
33 }
34 return null;
35}
36
37server.configure(function() {12server.configure(function() {
38 server.set('views', __dirname + './lib/views/');13 server.set('views', __dirname + './lib/views/');
39 server.set('view engine', 'handlebars');14 server.set('view engine', 'handlebars');
@@ -70,47 +45,27 @@
70 });45 });
71});46});
7247
73server.get('/juju-ui/assets/all-third.js', function(req, res) {48server.get('/juju-ui/assets/:file', function(req, res) {
74 if (debugMode) {49 var fileName = req.params.file;
75 res.sendfile('app/assets/javascripts/generated/all-third.js');50 if ('app.js' === fileName) {
76 } else {
77 res.sendfile('app/assets/javascripts/generated/all-third-min.js');
78 }
79});
80
81server.get('/juju-ui/assets/modules.js', function(req, res) {
82 if (debugMode) {
83 res.sendfile('app/assets/javascripts/generated/all-app-debug.js');
84 } else {
85 res.sendfile('app/assets/javascripts/generated/all-app.js');
86 }
87});
88
89server.get('/juju-ui/assets/yui.js', function(req, res) {
90 if (debugMode) {
91 res.sendfile('app/assets/javascripts/yui/yui/yui-debug.js');51 res.sendfile('app/assets/javascripts/yui/yui/yui-debug.js');
52 } else if ('modules.js' === fileName) {
53 res.sendfile('app/modules-debug.js');
54 } else if ('config.js' === fileName) {
55 res.sendfile('app/config.js');
92 } else {56 } else {
93 res.sendfile('app/assets/javascripts/generated/all-yui.js');57 res.sendfile('build/juju-ui/assets/' + fileName);
94 }58 }
95});59});
9660
61server.get('/juju-ui/:file', function(req, res) {
62 var fileName = req.params.file;
63 res.sendfile('build/juju-ui/' + fileName);
64});
65
97server.get('/juju-ui/assets/stylesheets/:file', function(req, res) {66server.get('/juju-ui/assets/stylesheets/:file', function(req, res) {
98 var file, fileName = req.params.file;67 var fileName = req.params.file;
99 if (path.extname(fileName).toLowerCase() === '.css') {68 res.sendfile('build/juju-ui/assets/stylesheets/' + fileName);
100 res.sendfile('app/assets/stylesheets/' + fileName);
101 } else {
102 // We've merged all the yui and third party files into a single css file.
103 // YUI expects to load its images from the same path as its css files, so
104 // we need to mock this position. When the system tries to load a resource
105 // from "app/assets/stylesheets/", it tries to find this resource under the
106 // "./app/assets" directory. The first match wins. Another solution would
107 // be to manually copy all the images we need to the
108 // "./app/assets/stylesheets" directory.
109 // This is an example of image...
110 // ./yui/datatable-sort/assets/skins/sam/sort-arrow-sprite-ie.png
111 file = findFile('./app/assets', fileName);
112 res.sendfile(file);
113 }
114});69});
11570
116server.get('*', function(req, res) {71server.get('*', function(req, res) {
11772
=== modified file 'lib/templates.js'
--- lib/templates.js 2012-09-26 23:31:13 +0000
+++ lib/templates.js 2012-11-19 20:22:19 +0000
@@ -122,7 +122,7 @@
122122
123var templateSpecs = {123var templateSpecs = {
124 templates: {124 templates: {
125 output: __dirname + '/../app/templates.js',125 output: __dirname + '/../build/juju-ui/templates.js',
126 callback: function(strategy, name) {126 callback: function(strategy, name) {
127 cache = {};127 cache = {};
128 getPrecompiled();128 getPrecompiled();
@@ -138,7 +138,7 @@
138 },138 },
139139
140 stylesheet: {140 stylesheet: {
141 output: __dirname + '/../app/assets/stylesheets/juju-gui.css',141 output: __dirname + '/../build/juju-ui/assets/stylesheets/juju-gui.css',
142 callback: function(strategy, name) {142 callback: function(strategy, name) {
143 var parser = new less.Parser({143 var parser = new less.Parser({
144 paths: [config.server.view_dir],144 paths: [config.server.view_dir],
145145
=== modified file 'test-server.js'
--- test-server.js 2012-10-31 10:57:16 +0000
+++ test-server.js 2012-11-19 20:22:19 +0000
@@ -17,7 +17,10 @@
17 server.use(express.methodOverride());17 server.use(express.methodOverride());
18});18});
1919
2020server.get('/juju-ui/:file', function(req, res) {
21 var fileName = req.params.file;
22 res.sendfile('build/juju-ui/' + fileName);
23});
2124
22var port = 8084;25var port = 8084;
2326
2427
=== modified file 'test/index.html'
--- test/index.html 2012-11-15 15:30:59 +0000
+++ test/index.html 2012-11-19 20:22:19 +0000
@@ -48,7 +48,7 @@
48 continue48 continue
49 }49 }
50 resource.fullpath = resource.fullpath.replace(50 resource.fullpath = resource.fullpath.replace(
51 '/juju-ui/', '../app/', 1);51 '/juju-ui/', '../juju-ui/', 1);
52 }52 }
53 }53 }
54 // Load before test runner54 // Load before test runner

Subscribers

People subscribed via source and target branches