Merge into trunk : makefile : Code : juju-gui

Status:	Merged
Merged at revision:	217
Proposed branch:	lp://staging/~gary/juju-gui/makefile
Merge into:	lp://staging/juju-gui/experimental
Diff against target:	148 lines (+43/-34) 4 files modified Makefile (+11/-8) README (+4/-0) app/assets/javascripts/svg-layouts.js (+17/-15) test-server.js (+11/-11)
To merge this branch:	bzr merge lp://staging/~gary/juju-gui/makefile
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Juju GUI Hackers		2012-10-31	Pending
Review via email: mp+132302@code.staging.launchpad.net

Description of the change

'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://codereview.appspot.com/6823057/

Revision history for this message

Gary Poster (gary) wrote on 2012-10-31:

#

Download full text (6.5 KiB)

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

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@canonical.com-20121031084626-es7gvcosfetv262g
+New revision: gary.poster@canonical.com-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']
  });

lp://staging/~gary/juju-gui/makefile updated on 2012-10-31

217. By Gary Poster on 2012-10-31

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

R=
CC=
https://codereview.appspot.com/6823057

Revision history for this message

Gary Poster (gary) wrote on 2012-10-31:

#

*** Submitted:

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

R=
CC=
https://codereview.appspot.com/6823057

https://codereview.appspot.com/6823057/

Revision history for this message

Nicola Larosa (teknico) wrote on 2012-10-31:

#

Looks good to me. Thanks for the single quotes. :-)

https://codereview.appspot.com/6823057/