yellow team mailing list archive

[Brad Crittenden <bac@xxxxxxxxxxxxx>]

Brad Crittenden has proposed merging lp:~bac/juju-gui/1078723 into lp:juju-gui.

Requested reviews:
  Juju GUI Hackers (juju-gui)
Related bugs:
  Bug #1078723 in juju-gui: "changes to third party files and bin/merge-files do not cause regeneration of combined files"
  https://bugs.launchpad.net/juju-gui/+bug/1078723

For more details, see:
https://code.launchpad.net/~bac/juju-gui/1078723/+merge/137991

Makefile tweaks

Bug 1078723 addressed problems with changes to third party JS not
forcing a remake of the combined JS files.  We really only have one
such file now, the others are handled properly, so this branch adds a
dependency on it, creates a THIRD_PARTY_JS symbol in the Makefile to
make it obvious in the future, and does other clean-up.

I tried removing the special-case for bin/generateTemplates (which
*is* a JavaScript file) by adding a .js extension.  That made most of
the make process work but then the linters blew up.  I hastily
retreated.

https://codereview.appspot.com/6873055/

-- 
https://code.launchpad.net/~bac/juju-gui/1078723/+merge/137991
Your team Juju GUI Hackers is requested to review the proposed merge of lp:~bac/juju-gui/1078723 into lp:juju-gui.

=== modified file 'Makefile'
--- Makefile	2012-12-04 17:32:59 +0000
+++ Makefile	2012-12-04 20:53:22 +0000
@@ -1,14 +1,19 @@
 # Makefile debugging hack: uncomment the two lines below and make will tell you
-# more about what is happening.  The output generated is of the form 
+# more about what is happening.  The output generated is of the form
 # "FILE:LINE [TARGET (DEPENDENCIES) (NEWER)]" where DEPENDENCIES are all the
 # things TARGET depends on and NEWER are all the files that are newer than
 # TARGET.  DEPENDENCIES will be colored green and NEWER will be blue.
 #
 #OLD_SHELL := $(SHELL)
-#SHELL = $(warning [$@ ($^) ($?) ])$(OLD_SHELL)
-
-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$$')
-
+#SHELL = $(warning [$@ ($^) ($?) ])$(OLD_SHELL)
+
+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$$')
+THIRD_PARTY_JS=app/assets/javascripts/reconnecting-websocket.js
 NODE_TARGETS=node_modules/chai node_modules/cryptojs node_modules/d3 \
 	node_modules/expect.js node_modules/express node_modules/graceful-fs \
 	node_modules/grunt node_modules/jshint node_modules/less \
@@ -31,6 +36,8 @@
 DATE=$(shell date -u)
 APPCACHE=$(BUILD_ASSETS_DIR)/manifest.appcache
 
+show:
+	echo $(JSFILES)
 all: build
 
 build/juju-ui/templates.js: $(TEMPLATE_TARGETS) bin/generateTemplates
@@ -121,7 +128,8 @@
 spritegen: $(SPRITE_GENERATED_FILES)
 
 $(PRODUCTION_FILES): node_modules/yui node_modules/d3/d3.v2.min.js $(JSFILES) \
-		bin/merge-files lib/merge-files.js
+		bin/merge-files lib/merge-files.js \
+		$(THIRD_PARTY_JS)
 	rm -f $(PRODUCTION_FILES)
 	mkdir -p "$(BUILD_ASSETS_DIR)/combined-css"
 	bin/merge-files
@@ -165,6 +173,7 @@
 
 $(BUILD_ASSETS_DIR)/images: $(SPRITE_SOURCE_FILES)
 	cp -rf app/assets/images $(BUILD_ASSETS_DIR)/images
+	touch $@
 
 $(BUILD_ASSETS_DIR)/svgs: $(shell bzr ls -R -k file app/assets/svgs)
 	cp -rf app/assets/svgs $(BUILD_ASSETS_DIR)/svgs

=== modified file 'bin/merge-files'
--- bin/merge-files	2012-12-04 17:32:59 +0000
+++ bin/merge-files	2012-12-04 20:53:22 +0000
@@ -66,7 +66,7 @@
     'app/assets/javascripts/d3-components.js',
     'app/assets/javascripts/reconnecting-websocket.js',
     'app/assets/javascripts/svg-layouts.js' ]);
-  
+
   merge.combineJs(filesToLoad.js, 'build/juju-ui/assets/all-yui.js');
 
   var cssFiles = filesToLoad.css;