yellow team mailing list archive

[Gary Poster <gary.poster@xxxxxxxxxxxxx>]

Reviewers: mp+132302_code.launchpad.net,

Message:
Please take a look.

Description:
'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

(do not edit description out of merge proposal)


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

Affected files:
   M Makefile
   M README
   A [revision details]
   M app/assets/javascripts/svg-layouts.js
   M test-server.js


Index: Makefile
=== modified file 'Makefile'
--- Makefile	2012-10-31 08:30:13 +0000
+++ Makefile	2012-10-31 10:57:16 +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


Index: README
=== modified file 'README'
--- README	2012-10-30 14:04:32 +0000
+++ README	2012-10-31 10:57:16 +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::



Index: [revision details]
=== added file '[revision details]'
--- [revision details]	2012-01-01 00:00:00 +0000
+++ [revision details]	2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: thiago.veronezi@xxxxxxxxxxxxx-20121031084626-es7gvcosfetv262g
+New revision: gary.poster@xxxxxxxxxxxxx-20121031105716-27xdngqrfg22ocd3

Index: test-server.js
=== modified file 'test-server.js'
--- test-server.js	2012-10-02 11:29:56 +0000
+++ test-server.js	2012-10-31 10:57:16 +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);
  });



Index: app/assets/javascripts/svg-layouts.js
=== 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 10:57:16 +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']
  });





-- 
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.