← Back to team overview

yellow team mailing list archive

[Merge] lp:~gary/juju-gui/makefile into lp:juju-gui

 

Gary Poster has proposed merging lp:~gary/juju-gui/makefile into lp:juju-gui.

Requested reviews:
  Juju GUI Hackers (juju-gui)

For more details, see:
https://code.launchpad.net/~gary/juju-gui/makefile/+merge/132302

'make server' should not always run yuidoc

I discovered that 'make server' always ran yuidoc.  This was because, in part, the automatically generated FILES list included inappropriate files.  There is another factor that affects this that I did not identify, but fixing the first problem is sufficient.

Fixing the problem specified the JS files more tightly, which revealed a few more lint changes.

I did a fly-by in the README.
-- 
https://code.launchpad.net/~gary/juju-gui/makefile/+merge/132302
Your team Juju GUI Hackers is requested to review the proposed merge of lp:~gary/juju-gui/makefile into lp:juju-gui.
=== modified file 'Makefile'
--- Makefile	2012-10-31 08:30:13 +0000
+++ Makefile	2012-10-31 11:24:23 +0000
@@ -1,4 +1,5 @@
-FILES=$(shell bzr ls -RV -k file | grep -v assets/ | grep -v app/templates.js | grep -v server.js)
+JSFILES=$(shell bzr ls -RV -k file | grep -E -e '.+\.js(on)?$$|generateTemplates$$' | grep -Ev -e '^manifest\.json$$' -e '^test/assets/' -e '^app/assets/javascripts/reconnecting-websocket.js$$' -e '^server.js$$')
+
 # After a successful "make" run, the NODE_TARGETS list can be regenerated with
 # this command (and then manually pasted in here):
 # find node_modules -maxdepth 1 -mindepth 1 -type d -printf 'node_modules/%f '
@@ -20,12 +21,14 @@
 app/templates.js: $(TEMPLATE_TARGETS) bin/generateTemplates
 	@./bin/generateTemplates
 
-yuidoc: install $(FILES)
+yuidoc/index.html: node_modules/yuidocjs $(JSFILES)
 	@node_modules/.bin/yuidoc -o yuidoc -x assets app
 
+yuidoc: yuidoc/index.html
+
 $(SPRITE_GENERATED_FILES): node_modules/grunt node_modules/node-spritesheet $(SPRITE_SOURCE_FILES)
 	@node_modules/grunt/bin/grunt spritegen
-	mv bin/sprite app/assets/sprite/
+	@mv bin/sprite app/assets/sprite/
 
 $(NODE_TARGETS): package.json
 	@npm install
@@ -38,12 +41,12 @@
 gjslint: virtualenv/bin/gjslint
 	@virtualenv/bin/gjslint --strict --nojsdoc --jslint_error=all \
 	    --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 \
-	    $(FILES)
+	    $(JSFILES)
 
 jshint: node_modules/jshint
-	@node_modules/jshint/bin/hint $(FILES)
+	@node_modules/jshint/bin/hint $(JSFILES)
 
-yuidoc-lint: $(FILES)
+yuidoc-lint: $(JSFILES)
 	@bin/lint-yuidoc
 
 lint: gjslint jshint yuidoc-lint
@@ -53,7 +56,7 @@
 	@virtualenv/bin/easy_install archives/closure_linter-latest.tar.gz
 
 beautify: virtualenv/bin/fixjsstyle
-	@virtualenv/bin/fixjsstyle --strict --nojsdoc --jslint_error=all $(FILES)
+	@virtualenv/bin/fixjsstyle --strict --nojsdoc --jslint_error=all $(JSFILES)
 
 spritegen: $(SPRITE_GENERATED_FILES)
 
@@ -87,4 +90,4 @@
 appcache-force: appcache-touch appcache
 
 .PHONY: test lint beautify server install clean prep jshint gjslint \
-	appcache appcache-touch appcache-force spritegen yuidoc-lint
+	appcache appcache-touch appcache-force yuidoc spritegen yuidoc-lint

=== modified file 'README'
--- README	2012-10-30 14:04:32 +0000
+++ README	2012-10-31 11:24:23 +0000
@@ -9,6 +9,10 @@
   $ sudo apt-get update
   $ sudo apt-get install nodejs npm
 
+You also need ImageMagick.
+
+  $ sudo apt-get install imagemagick
+
 The linter will be installed locally per branch, but if you want editor
 integration, you may want to install jshint globally in your system::
 

=== modified file 'app/assets/javascripts/svg-layouts.js'
--- app/assets/javascripts/svg-layouts.js	2012-09-14 04:35:36 +0000
+++ app/assets/javascripts/svg-layouts.js	2012-10-31 11:24:23 +0000
@@ -1,16 +1,18 @@
-YUI.add("svg-layouts", function(Y) {
-    // package to help compute svg layouts,
-    // particuallary around text
-    Y.Node.addMethod("getClientRect", function(node) {
-        // Chrome and FF both support this at the DOM level
-        var rect = node.getClientRects()[0];
-        if (typeof(rect) == "undefined" || !rect.width) {
-            return null;
-        }
-        return rect;
-    });
-            
-
-}, "0.0.1", {
-    requires: ["node"]
+'use strict';
+
+YUI.add('svg-layouts', function(Y) {
+  // package to help compute svg layouts,
+  // particuallary around text
+  Y.Node.addMethod('getClientRect', function(node) {
+    // Chrome and FF both support this at the DOM level
+    var rect = node.getClientRects()[0];
+    if (typeof(rect) === 'undefined' || !rect.width) {
+      return null;
+    }
+    return rect;
+  });
+
+
+}, '0.0.1', {
+  requires: ['node']
 });

=== modified file 'test-server.js'
--- test-server.js	2012-10-02 11:29:56 +0000
+++ test-server.js	2012-10-31 11:24:23 +0000
@@ -6,22 +6,22 @@
     path = require('path');
 
 
-server.configure(function () {
-    server.use(express.logger('dev'));
-    // 'static' is a reserved word so dot notation is not used to
-    // avoid annoying the linter.
-    server.use(express['static'](__dirname));
-    // fallback to looking in assets
-    server.use('/juju-ui', express['static'](__dirname + '/app/'));
-    server.use(express.bodyParser());
-    server.use(express.methodOverride());
+server.configure(function() {
+  server.use(express.logger('dev'));
+  // 'static' is a reserved word so dot notation is not used to
+  // avoid annoying the linter.
+  server.use(express['static'](__dirname));
+  // fallback to looking in assets
+  server.use('/juju-ui', express['static'](__dirname + '/app/'));
+  server.use(express.bodyParser());
+  server.use(express.methodOverride());
 });
 
 
 
 var port = 8084;
 
-server.listen(port, function () {
-    console.log('Server listening on ' + port);
+server.listen(port, function() {
+  console.log('Server listening on ' + port);
 });
 


Follow ups