Merge lp:~bac/juju-gui/1078723 into lp:juju-gui/experimental

[Brad Crittenden]

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/

[Brad Crittenden (bac)]

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

[Francesco Banconi (frankban)]

Approved.
These changes look good Brad, thanks.

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

[lp:~bac/juju-gui/1078723]

268. By Brad Crittenden on 2012-12-05

Remove debug line before commit

[Brad Crittenden (bac)]

*** Submitted:

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.

R=benji, frankban
CC=
https://codereview.appspot.com/6873055

https://codereview.appspot.com/6873055/diff/1/Makefile
File Makefile (right):

https://codereview.appspot.com/6873055/diff/1/Makefile#newcode40
Makefile:40: echo $(JSFILES)
gah!
gone.
done.

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

=== modified file 'Makefile'
--- Makefile	2012-12-04 22:57:18 +0000
+++ Makefile	2012-12-05 12:55:26 +0000
@@ -1,5 +1,24 @@
+<<<<<<< TREE
 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$$')
 
+=======
+# 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
+# "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$$')
+THIRD_PARTY_JS=app/assets/javascripts/reconnecting-websocket.js
+>>>>>>> MERGE-SOURCE
 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 \
@@ -108,7 +127,13 @@
 
 spritegen: $(SPRITE_GENERATED_FILES)
 
+<<<<<<< TREE
 $(PRODUCTION_FILES): node_modules/yui node_modules/d3/d3.v2.min.js $(JSFILES) ./bin/merge-files
+=======
+$(PRODUCTION_FILES): node_modules/yui node_modules/d3/d3.v2.min.js $(JSFILES) \
+		bin/merge-files lib/merge-files.js \
+		$(THIRD_PARTY_JS)
+>>>>>>> MERGE-SOURCE
 	rm -f $(PRODUCTION_FILES)
 	mkdir -p "$(BUILD_ASSETS_DIR)/stylesheets"
 	./bin/merge-files
@@ -143,6 +168,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 22:57:18 +0000
+++ bin/merge-files	2012-12-05 12:55:26 +0000
@@ -61,6 +61,7 @@
 
   // Merge third-party files to the filesToLoad list
   filesToLoad.js.push.apply(filesToLoad.js, [
+<<<<<<< TREE
       './app/assets/javascripts/d3.v2.min.js',
       './app/assets/javascripts/d3-components.js',
       './app/assets/javascripts/reconnecting-websocket.js',
@@ -77,4 +78,18 @@
   merge.combine([ './app/assets/stylesheets/bootstrap-2.0.4.css',
       './app/assets/stylesheets/bootstrap-responsive-2.0.4.css' ],
       './build/juju-ui/assets/stylesheets/all-static.css', 'yui-css');
+=======
+    'app/assets/javascripts/d3.v2.min.js',
+    '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;
+  cssFiles.push('app/assets/stylesheets/bootstrap-2.0.4.css');
+  cssFiles.push('app/assets/stylesheets/bootstrap-responsive-2.0.4.css');
+  merge.combineCSS(cssFiles,
+    'build/juju-ui/assets/combined-css/all-static.css');
+>>>>>>> MERGE-SOURCE
 });