launchpad-reviewers team mailing list archive

[Richard Harding <rick.harding@xxxxxxxxxxxxx>]

Richard Harding has proposed merging lp:~rharding/launchpad/yuiv5 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~rharding/launchpad/yuiv5/+merge/119406

= Summary =

This is another attempt to get side by side versions of YUI to install and
hook into the existing yui_version feature flag.

== Pre Implementation ==

Discussed moving things out of the buildout with lifeless. We're hitting
friction with having one version of YUI in versions.cfg (which is for python
packages) and doing multiple versions of YUI at once for testing.

== Implementation Notes ==

We move the versions of YUI supported into the Makefile as YUI_VERSIONS.
There's a default version which is symlinked to the usual build/js/yui target.
This get overridden by the feature flag and allows setting other versions
we've got packaged. Currently, we only allow 3.3.0 (default) and 3.5.1.

The download cache contains both versions, all systems have unzip installed as
part of lp-deps.

== Q/A ==

Make sure we can switch to using the 3.5.1 JS via the feature flag below and see all the combo load urls sprout paths in the query string that mention 3.5.1 and yet still return blobs of JS.

js.yui_version    default    1    yui-3.5.1

Note: at the start of this work all JS tests were made to pass with 3.5.1.
Since this has taken time to get going, some tests are failing.

https://bugs.launchpad.net/launchpad/+bug/1036313

Since this work is behind a feature flag, it is safe to land and the list of
failing tests can be updated as a next step.

== Tests ==

All tests currently pass with YUI 3.3.0. Note that our current test system
only runs tests against the default YUI version. Tests against the other
versions must be manually run by altering the build/js/yui symlink to the
version you want to test.

== LoC Qualification ==

Just barely negative LoC
-- 
https://code.launchpad.net/~rharding/launchpad/yuiv5/+merge/119406
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rharding/launchpad/yuiv5 into lp:launchpad.

=== modified file 'Makefile'
--- Makefile	2012-08-09 06:46:51 +0000
+++ Makefile	2012-08-13 18:31:24 +0000
@@ -34,6 +34,10 @@
 ! -path 'lib/lp/services/*'
 endef
 
+JS_BUILD_DIR := build/js
+YUI_VERSIONS := 3.3.0 3.5.1
+YUI_BUILDS := $(patsubst %,$(JS_BUILD_DIR)/yui-%, $(YUI_VERSIONS))
+YUI_DEFAULT := $(JS_BUILD_DIR)/yui-3.3.0
 JS_YUI := $(shell utilities/yui-deps.py $(JS_BUILD:raw=))
 JS_LP := $(shell find -L $(JS_LP_PATHS) -name '*.js' ! -name '.*.js')
 JS_ALL := $(JS_YUI) $(JS_LP)
@@ -171,10 +175,24 @@
 jsbuild_watch:
 	$(PY) bin/watch_jsbuild
 
+$(JS_BUILD_DIR):
+	mkdir -p $@
+
+$(YUI_BUILDS): $(JS_BUILD_DIR)
+	for V in $(YUI_VERSIONS); do \
+		mkdir $(JS_BUILD_DIR)/yui-$$V $(JS_BUILD_DIR)/yui-$$V-tmp; \
+		unzip -q download-cache/dist/yui_$$V.zip -d $(JS_BUILD_DIR)/yui-$$V-tmp; \
+		mv $(JS_BUILD_DIR)/yui-$$V-tmp/yui/build/* $(JS_BUILD_DIR)/yui-$$V/; \
+		rm -rf $(JS_BUILD_DIR)/yui-$$V-tmp; \
+	done
+
 $(JS_LP): jsbuild_widget_css
 $(JS_YUI):
 	cp -a lib/canonical/launchpad/icing/yui_2.7.0b/build build/js/yui2
 
+# YUI_DEFAULT is one of the targets in YUI_BUILDS which expands all of our YUI
+# versions for us.
+$(JS_ALL): $(YUI_DEFAULT)
 $(JS_OUT): $(JS_ALL)
 ifeq ($(JS_BUILD), min)
 	cat $^ | bin/lpjsmin > $@
@@ -187,7 +205,7 @@
 	utilities/check-js-deps
 
 jsbuild: $(PY) $(JS_OUT)
-	bin/combo-rootdir build/js
+	bin/combo-rootdir build/js $(YUI_DEFAULT)
 
 eggs:
 	# Usually this is linked via link-external-sourcecode, but in
@@ -470,7 +488,7 @@
 	if [ ! -d /srv/launchpad.dev ]; then \
 		mkdir /srv/launchpad.dev; \
 		chown $(SUDO_UID):$(SUDO_GID) /srv/launchpad.dev; \
-	fi	
+	fi
 
 enable-apache-launchpad: copy-apache-config copy-certificates
 	a2ensite local-launchpad

=== modified file 'buildout-templates/bin/combo-rootdir.in'
--- buildout-templates/bin/combo-rootdir.in	2012-07-13 01:55:27 +0000
+++ buildout-templates/bin/combo-rootdir.in	2012-08-13 18:31:24 +0000
@@ -10,11 +10,17 @@
     exit 1
 fi
 
+if [ -z "$2" ]; then
+    echo "$0: Need yui build version to use as default as second argument"
+    exit 1
+fi
+
 BUILD_DIR=$1
+YUI_VERSION=$2
 
 # Populate YUI.
 if [ ! -h $BUILD_DIR/yui ]; then
-    ln -sf yui-${versions:yui} $BUILD_DIR/yui
+    ln -sf `basename $YUI_VERSION` $BUILD_DIR/yui
 fi
 
 # And YUI 2, for now.

=== modified file 'buildout.cfg'
--- buildout.cfg	2012-07-25 21:09:38 +0000
+++ buildout.cfg	2012-08-13 18:31:24 +0000
@@ -3,7 +3,6 @@
 
 [buildout]
 parts =
-    yui-default
     scripts
     filetemplates
     tags
@@ -14,7 +13,6 @@
 unzip = true
 eggs-directory = eggs
 download-cache = download-cache
-yui-directory = build/js
 relative-paths = true
 
 # Disable this option temporarily if you want buildout to find software
@@ -37,28 +35,6 @@
 [configuration]
 instance_name = development
 
-[yui]
-recipe = plone.recipe.command
-command =
-    mkdir -p ${buildout:yui-directory}/yui-${:yui_version}
-    rm -rf ${buildout:yui-directory}/yui-${:yui_version}/*
-    tar -zxf download-cache/dist/yui-${:yui_version}.tar.gz \
-        --wildcards --strip-components 2 \
-        -C ${buildout:yui-directory}/yui-${:yui_version} \
-        '*/build'
-
-[yui-default]
-<= yui
-yui_version = ${versions:yui}
-
-[yui-3.4]
-<= yui
-yui_version = 3.4.1
-
-[yui-3.5]
-<=yui
-yui_version = 3.5.0pr1
-
 [filetemplates]
 recipe = z3c.recipe.filetemplate
 source-directory = buildout-templates

=== modified file 'lib/lp/app/templates/base-layout-macros.pt'
--- lib/lp/app/templates/base-layout-macros.pt	2012-08-10 04:48:36 +0000
+++ lib/lp/app/templates/base-layout-macros.pt	2012-08-13 18:31:24 +0000
@@ -88,8 +88,7 @@
       tal:content="string:var cookie_scope = '${request/lp:cookie_scope}';"></script>
 
   <tal:js-loader condition="request/features/js.combo_loader.enabled">
-   <script type="text/javascript"
-        tal:attributes="src string:${combo_url}/?yui/yui/yui-min.js&amp;lp/meta.js&amp;yui/loader/loader-min.js"></script>
+   <script type="text/javascript" tal:attributes="src string:${combo_url}/?${yui_version}/yui/yui-min.js&amp;lp/meta.js&amp;${yui_version}/loader/loader-min.js"></script>
    <script type="text/javascript" tal:content="string:
         var raw = null;
         if (LP.devmode) {
@@ -98,10 +97,11 @@
         YUI.GlobalConfig = {
             combine: true,
             comboBase: '${combo_url}/?',
-            root: 'yui/',
+            root: '${yui_version}/',
             filter: raw,
             debug: ${yui_console_debug},
             fetchCSS: false,
+            maxURLLength: 2000,
             groups: {
                 lp: {
                     combine: true,

=== modified file 'versions.cfg'
--- versions.cfg	2012-08-10 18:10:08 +0000
+++ versions.cfg	2012-08-13 18:31:24 +0000
@@ -146,7 +146,6 @@
 wsgi-jsonrpc = 0.2.8
 wsgi-xmlrpc = 0.2.7
 wsgiref = 0.1.2
-yui = 3.3.0
 z3c.coverage = 1.1.2
 z3c.csvvocabulary = 1.0.0
 z3c.etestbrowser = 1.0.4