yellow team mailing list archive

[Brad Crittenden <bac@xxxxxxxxxxxxx>]

Reviewers: mp+137991_code.launchpad.net,

Message:
Please take a look.

Description:
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://code.launchpad.net/~bac/juju-gui/1078723/+merge/137991

(do not edit description out of merge proposal)


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

Affected files:
   M Makefile
   A [revision details]
   M bin/merge-files


Index: Makefile
=== modified file 'Makefile'
--- Makefile	2012-12-04 17:32:59 +0000
+++ Makefile	2012-12-04 20:48:08 +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


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: benji.york@xxxxxxxxxxxxx-20121204200919-fx3717n9o7rqm6zh
+New revision: bac@xxxxxxxxxxxxx-20121204204808-3u9radx15g5ha9ij

Index: bin/merge-files
=== modified file 'bin/merge-files'
--- bin/merge-files	2012-12-04 17:32:59 +0000
+++ bin/merge-files	2012-12-04 20:41:38 +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;





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