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: [email protected] +New revision: [email protected] 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. -- Mailing list: https://launchpad.net/~yellow Post to : [email protected] Unsubscribe : https://launchpad.net/~yellow More help : https://help.launchpad.net/ListHelp
