yellow team mailing list archive

[Thiago Veronezi <thiago@xxxxxxxxxxxx>]

Thiago Veronezi has proposed merging lp:~tveronezi/juju-gui/make-build into lp:juju-gui.

Requested reviews:
  Juju GUI Hackers (juju-gui)
Related bugs:
  Bug #1078898 in juju-gui: "Makefile does not support static file deployment"
  https://bugs.launchpad.net/juju-gui/+bug/1078898

For more details, see:
https://code.launchpad.net/~tveronezi/juju-gui/make-build/+merge/134706

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/

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

=== modified file '.bzrignore'
--- .bzrignore	2012-11-13 15:04:19 +0000
+++ .bzrignore	2012-11-16 16:35:27 +0000
@@ -3,22 +3,14 @@
 gui.sublime-project
 .emacs.desktop.lock
 gui.sublime-workspace
-app/templates.js
-app/assets/stylesheets/juju-gui.css
 app/assets/javascripts/d3.v2.js
 app/assets/javascripts/d3.v2.min.js
 app/assets/javascripts/yui
-app/templates.js
-app/assets/stylesheets/juju-gui.css
 docs/_build/doctrees
 docs/_build/html
 *~
-app/assets/stylesheets/juju-gui.css
 tags
 Session.vim
 virtualenv
 yuidoc
-app/assets/manifest.appcache
-app/assets/sprite
-app/assets/javascripts/generated/
-app/assets/stylesheets/all-static.css
+build

=== modified file 'Makefile'
--- Makefile	2012-11-15 13:07:29 +0000
+++ Makefile	2012-11-16 16:35:27 +0000
@@ -13,18 +13,19 @@
 	node_modules/node-minify
 TEMPLATE_TARGETS=$(shell bzr ls -k file app/templates)
 SPRITE_SOURCE_FILES=$(shell bzr ls -R -k file app/assets/images)
-SPRITE_GENERATED_FILES=app/assets/sprite/sprite.css app/assets/sprite/sprite.png
-COMPRESSED_FILES=app/assets/javascripts/generated/all-app-debug.js \
-	app/assets/javascripts/generated/all-app.js \
-	app/assets/javascripts/generated/all-third.js \
-	app/assets/javascripts/generated/all-yui.js \
-	app/assets/stylesheets/all-static.css
+SPRITE_GENERATED_FILES=build/juju-ui/assets/stylesheets/sprite.css \
+	build/juju-ui/assets/stylesheets/sprite.png
+PRODUCTION_FILES=build/juju-ui/assets/modules.js \
+	build/juju-ui/assets/config.js \
+	build/juju-ui/assets/app.js \
+	build/juju-ui/assets/stylesheets/all-static.css
 DATE=$(shell date -u)
-APPCACHE=app/assets/manifest.appcache
+APPCACHE=build/juju-ui/assets/manifest.appcache
 
 all: install
 
-app/templates.js: $(TEMPLATE_TARGETS) bin/generateTemplates
+build/juju-ui/templates.js: $(TEMPLATE_TARGETS) bin/generateTemplates
+	@test -d "build/juju-ui/assets/stylesheets" || mkdir -p "build/juju-ui/assets/stylesheets"
 	@./bin/generateTemplates
 
 yuidoc/index.html: node_modules/yuidocjs $(JSFILES)
@@ -34,8 +35,6 @@
 
 $(SPRITE_GENERATED_FILES): node_modules/grunt node_modules/node-spritesheet $(SPRITE_SOURCE_FILES)
 	@node_modules/grunt/bin/grunt spritegen
-	@rm -Rf app/assets/sprite/
-	@mv bin/sprite app/assets
 
 $(NODE_TARGETS): package.json
 	@npm install
@@ -43,7 +42,7 @@
 	@ln -sf `pwd`/node_modules/yui ./app/assets/javascripts/
 	@ln -sf `pwd`/node_modules/d3/d3.v2* ./app/assets/javascripts/
 
-install: appcache $(NODE_TARGETS) app/templates.js yuidoc spritegen combinejs
+install: appcache $(NODE_TARGETS) build/juju-ui/templates.js yuidoc spritegen combinejs
 
 gjslint: virtualenv/bin/gjslint
 	@virtualenv/bin/gjslint --strict --nojsdoc --jslint_error=all \
@@ -67,13 +66,14 @@
 
 spritegen: $(SPRITE_GENERATED_FILES)
 
-$(COMPRESSED_FILES): node_modules/yui node_modules/d3/d3.v2.min.js $(JSFILES) ./bin/merge-files
-	@rm -f app/assets/stylesheets/all-static.css
-	@rm -Rf app/assets/javascripts/generated/
-	@mkdir app/assets/javascripts/generated/
+$(PRODUCTION_FILES): node_modules/yui node_modules/d3/d3.v2.min.js $(JSFILES) ./bin/merge-files
+	@rm -f $(PRODUCTION_FILES)
+	@test -d "build/juju-ui/assets/stylesheets" || mkdir -p "build/juju-ui/assets/stylesheets"
 	@./bin/merge-files
+	@cp app/modules.js build/juju-ui/assets/modules.js
+	@cp app/config.js build/juju-ui/assets/config.js
 
-combinejs: $(COMPRESSED_FILES)
+combinejs: $(PRODUCTION_FILES)
 
 prep: beautify lint
 
@@ -82,21 +82,24 @@
 
 debug: install
 	@echo "Customize config.js to modify server settings"
-	@node server.js debug
-
-server: install
-	@echo "Customize config.js to modify server settings"
 	@node server.js
 
+server: build
+	@echo "Runnning the application from a SimpleHTTPServer"
+	@cd build && python -m SimpleHTTPServer 8888
+
 clean:
 	@rm -rf node_modules virtualenv
 	@make -C docs clean
-	@rm -Rf bin/sprite/
-	@rm -Rf app/assets/sprite/
-	@rm -Rf app/assets/javascripts/generated/
-	@rm -f app/assets/stylesheets/all-static.css
+	@rm -Rf build/
+
+build: install
+	@cp -f app/index.html build/
+	@cp -rf app/assets/images build/juju-ui/assets/images
+	@cp -rf app/assets/svgs build/juju-ui/assets/svgs
 
 $(APPCACHE): manifest.appcache.in
+	@test -d "build/juju-ui/assets" || mkdir -p "build/juju-ui/assets"
 	@cp manifest.appcache.in $(APPCACHE)
 	@sed -re 's/^\# TIMESTAMP .+$$/\# TIMESTAMP $(DATE)/' -i $(APPCACHE)
 
@@ -112,4 +115,4 @@
 
 .PHONY: test lint beautify server install clean prep jshint gjslint \
 	appcache appcache-touch appcache-force yuidoc spritegen yuidoc-lint \
-	combinejs
+	combinejs build

=== modified file 'app/index.html'
--- app/index.html	2012-11-15 15:44:00 +0000
+++ app/index.html	2012-11-16 16:35:27 +0000
@@ -9,7 +9,10 @@
     <link href='http://fonts.googleapis.com/css?family=Ubuntu:400,400italic&subset=latin,latin-ext' rel='stylesheet' type='text/css'>
     <link rel="stylesheet" href="/juju-ui/assets/stylesheets/all-static.css">
     <link rel="stylesheet" href="/juju-ui/assets/stylesheets/juju-gui.css">
-    <link rel="stylesheet" href="/juju-ui/assets/sprite/sprite.css">
+    <link rel="stylesheet" href="/juju-ui/assets/stylesheets/sprite.css">
+    <!-- When we merge the YUI js files, YUI does not load it automatically. -->
+    <link rel="stylesheet" href="http://yui.yahooapis.com/combo?3.8.0pr1/build/slider-base/assets/skins/sam/slider-base.css";>
+    <link>
     <!--[if lt IE 9]>
     <script src="http://html5shim.googlecode.com/svn/trunk/html5.js";></script>
     <![endif]-->
@@ -68,9 +71,9 @@
         <div id="vp-right-border"></div>
     </div>
     <!-- javascript away -->
-    <script src="/juju-ui/assets/yui.js"></script>
-    <script src="/juju-ui/assets/all-third.js"></script>
+    <script src="/juju-ui/assets/app.js"></script>
     <script src="/juju-ui/assets/modules.js"></script>
+    <script src="/juju-ui/assets/config.js"></script>
     <script>
       YUI(GlobalConfig).use(["juju-gui"], function(Y) {
         app = new Y.juju.App(juju_config);

=== modified file 'bin/merge-files'
--- bin/merge-files	2012-11-15 13:07:29 +0000
+++ bin/merge-files	2012-11-16 16:35:27 +0000
@@ -30,8 +30,7 @@
       syspath = require('path'),
       paths,
       reqs,
-      filesToLoad,
-      thirdPartyCSS;
+      filesToLoad;
 
   // First we find all of the paths to our custom Javascript in the app
   // directory.  We need to tell the function to ignore the "assets" directory
@@ -42,6 +41,9 @@
     [syspath.join(process.cwd(), './app/assets'),
      syspath.join(process.cwd(), './app/modules-debug.js')]);
 
+  // templates.js is a generated file. It is not part of the app directory.
+  paths.push(syspath.join(process.cwd(), './build/juju-ui/templates.js'));
+
   // Get the name of all the YUI modules that our custom js files use directly.
   reqs = merge.loadRequires(paths);
 
@@ -56,41 +58,22 @@
 
   // Get all of the YUI files and their dependencies
   filesToLoad = merge.getYUIFiles(reqs);
-  thirdPartyCSS = filesToLoad.css;
-  thirdPartyCSS.push('./app/assets/stylesheets/bootstrap-2.0.4.css');
-  thirdPartyCSS.push('./app/assets/stylesheets/bootstrap-responsive-2.0.4.css');
-  console.log('CSS: ' + thirdPartyCSS);
-
-  // Combine YUI js files
+
+  // Merge third-party files to the filesToLoad list
+  filesToLoad.js.push.apply(filesToLoad.js, [ './app/assets/javascripts/d3.v2.min.js',
+      './app/assets/javascripts/d3-components.js',
+      './app/assets/javascripts/reconnecting-websocket.js',
+      './app/assets/javascripts/svg-layouts.js' ]);
+  
+  // Merge our list of custom js files to the filesToLoad list
+  filesToLoad.js.push.apply(filesToLoad.js, paths);
+
+  // Combine YUI and our js files
   merge.combine(filesToLoad.js,
-      './app/assets/javascripts/generated/all-yui.js', 'uglifyjs');
+      './build/juju-ui/assets/app.js', 'uglifyjs');
 
   // Combine third party css files
-  merge.combine(thirdPartyCSS,
-      './app/assets/stylesheets/all-static.css', 'yui-css');
-
-  // Combine third party js libraries
-  merge.combine([ './app/assets/javascripts/d3.v2.min.js',
-      './app/assets/javascripts/d3-components.js',
-      './app/assets/javascripts/reconnecting-websocket.js',
-      './app/assets/javascripts/svg-layouts.js' ],
-      './app/assets/javascripts/generated/all-third-min.js', 'uglifyjs');
-
-  // It has only one file but eventually it will have more.
-  merge.combine([ './app/assets/javascripts/d3.v2.js' ],
-      './app/assets/javascripts/generated/all-third.js', 'uglifyjs');
-
-  // Now we only need to generate the file that is used to tell YUI where all
-  // the dependencies are.  We either use a debug version or the production
-  // version.  The debug version is simply pointers to the individual files.
-  // The production version aggregates all of the files together.
-
-  // Creating the combined file for the modules-debug.js and config.js files
-  merge.combine(['./app/modules-debug.js', './app/config.js'],
-      './app/assets/javascripts/generated/all-app-debug.js');
-
-  // Creating the combined file for all our files.  Note that this includes
-  // app/modules.js in the rollup, which is why that file still needs exist.
-  merge.combine(paths, './app/assets/javascripts/generated/all-app.js',
-      'uglifyjs');
+  merge.combine([ './app/assets/stylesheets/bootstrap-2.0.4.css',
+      './app/assets/stylesheets/bootstrap-responsive-2.0.4.css' ],
+      './build/juju-ui/assets/stylesheets/all-static.css', 'yui-css');
 });

=== modified file 'grunt.js'
--- grunt.js	2012-11-06 12:28:51 +0000
+++ grunt.js	2012-11-16 16:35:27 +0000
@@ -5,13 +5,13 @@
     spritesheet: {
       compile: {
         options: {
-          outputImage: 'sprite/sprite.png',
-          outputCss: 'sprite/sprite.css',
+          outputImage: 'stylesheets/sprite.png',
+          outputCss: 'stylesheets/sprite.css',
           selector: '.sprite'
 
         },
         files: {
-          'bin': 'app/assets/images/*'
+          'build/juju-ui/assets': 'app/assets/images/*'
         }
       }
     }

=== modified file 'lib/server.js'
--- lib/server.js	2012-11-15 13:07:29 +0000
+++ lib/server.js	2012-11-16 16:35:27 +0000
@@ -5,35 +5,10 @@
     fs = require('fs'),
     path = require('path'),
     config = require('../config').config.server,
-    debugMode = (String(process.argv[2]).toLowerCase() === 'debug'),
     public_dir = config.public_dir,
     Templates = require('./templates.js'),
     view = require('./view.js');
 
-
-/**
- * It finds a file under the given path. The first match wins.
- * @param {string} path starting point.
- * @param {string} fileName name of the file to be found.
- * @return {string} the file path.
- */
-function findFile(path, fileName) {
-  var file, files = fs.readdirSync(path);
-  for (var i = 0; i < files.length; i = i + 1) {
-    if (files[i] === fileName) {
-      return path + '/' + files[i];
-    }
-    file = fs.statSync(path + '/' + files[i]);
-    if (file.isDirectory()) {
-      file = findFile(path + '/' + files[i], fileName);
-      if (file) {
-        return file;
-      }
-    }
-  }
-  return null;
-}
-
 server.configure(function() {
   server.set('views', __dirname + './lib/views/');
   server.set('view engine', 'handlebars');
@@ -70,47 +45,27 @@
   });
 });
 
-server.get('/juju-ui/assets/all-third.js', function(req, res) {
-  if (debugMode) {
-    res.sendfile('app/assets/javascripts/generated/all-third.js');
-  } else {
-    res.sendfile('app/assets/javascripts/generated/all-third-min.js');
-  }
-});
-
-server.get('/juju-ui/assets/modules.js', function(req, res) {
-  if (debugMode) {
-    res.sendfile('app/assets/javascripts/generated/all-app-debug.js');
-  } else {
-    res.sendfile('app/assets/javascripts/generated/all-app.js');
-  }
-});
-
-server.get('/juju-ui/assets/yui.js', function(req, res) {
-  if (debugMode) {
+server.get('/juju-ui/assets/:file', function(req, res) {
+  var fileName = req.params.file;
+  if ('app.js' === fileName) {
     res.sendfile('app/assets/javascripts/yui/yui/yui-debug.js');
+  } else if ('modules.js' === fileName) {
+    res.sendfile('app/modules-debug.js');
+  } else if ('config.js' === fileName) {
+    res.sendfile('app/config.js');
   } else {
-    res.sendfile('app/assets/javascripts/generated/all-yui.js');
+    res.sendfile('build/juju-ui/assets/' + fileName);
   }
 });
 
+server.get('/juju-ui/:file', function(req, res) {
+  var fileName = req.params.file;
+  res.sendfile('build/juju-ui/' + fileName);
+});
+
 server.get('/juju-ui/assets/stylesheets/:file', function(req, res) {
-  var file, fileName = req.params.file;
-  if (path.extname(fileName).toLowerCase() === '.css') {
-    res.sendfile('app/assets/stylesheets/' + fileName);
-  } else {
-    // We've merged all the yui and third party files into a single css file.
-    // YUI expects to load its images from the same path as its css files, so
-    // we need to mock this position. When the system tries to load a resource
-    // from "app/assets/stylesheets/", it tries to find this resource under the
-    // "./app/assets" directory. The first match wins. Another solution would
-    // be to manually copy all the images we need to the
-    // "./app/assets/stylesheets" directory.
-    // This is an example of image...
-    // ./yui/datatable-sort/assets/skins/sam/sort-arrow-sprite-ie.png
-    file = findFile('./app/assets', fileName);
-    res.sendfile(file);
-  }
+  var fileName = req.params.file;
+  res.sendfile('build/juju-ui/assets/stylesheets/' + fileName);
 });
 
 server.get('*', function(req, res) {

=== modified file 'lib/templates.js'
--- lib/templates.js	2012-09-26 23:31:13 +0000
+++ lib/templates.js	2012-11-16 16:35:27 +0000
@@ -122,7 +122,7 @@
 
 var templateSpecs = {
   templates: {
-    output: __dirname + '/../app/templates.js',
+    output: __dirname + '/../build/juju-ui/templates.js',
     callback: function(strategy, name) {
       cache = {};
       getPrecompiled();
@@ -138,7 +138,7 @@
   },
 
   stylesheet: {
-    output: __dirname + '/../app/assets/stylesheets/juju-gui.css',
+    output: __dirname + '/../build/juju-ui/assets/stylesheets/juju-gui.css',
     callback: function(strategy, name) {
       var parser = new less.Parser({
         paths: [config.server.view_dir],