← Back to team overview

launchpad-reviewers team mailing list archive

Re: [Merge] lp:~cjwatson/launchpad/yarn into lp:launchpad

 

xenial's nodejs is sufficient for our purposes.  I'm inclined to wait until the upgrades are done rather than bothering with a backport.

Diff comments:

> 
> === modified file 'Makefile'
> --- Makefile	2017-05-11 14:15:36 +0000
> +++ Makefile	2017-07-20 17:52:40 +0000
> @@ -159,17 +159,20 @@
>  $(JS_BUILD_DIR):
>  	mkdir -p $@
>  
> -$(YUI_BUILDS): | $(JS_BUILD_DIR)
> +$(YARN_BUILD): | $(JS_BUILD_DIR)
>  	mkdir -p $@/tmp
> -	unzip -q download-cache/dist/yui_$(subst build/js/yui-,,$@).zip -d $@/tmp 'yui/build/*'
> -	# We don't use the Flash components and they have a bad security
> +	tar -C $@/tmp -xf download-cache/dist/yarn-$(YARN_VERSION).tar.gz
> +	mv $@/tmp/dist/* $@
> +	$(RM) -r $@/tmp
> +
> +node_modules/yui: package.json | $(YARN_BUILD)
> +	$(YARN) install --offline --frozen-lockfile

Well, yes, on its own it's not obviously useful, but I think it will be clearer once we get to do things like replacing in-tree copies of random bits of JS code with proper external requirements.

> +	# We don't use YUI's Flash components and they have a bad security
>  	# record. Kill them.
> -	find $@/tmp/yui/build -name '*.swf' -delete
> -	mv $@/tmp/yui/build/* $@
> -	$(RM) -r $@/tmp
> +	find node_modules/yui -name '*.swf' -delete
>  
> -$(YUI_DEFAULT_SYMLINK): $(YUI_BUILDS)
> -	ln -sfn $(YUI_DEFAULT) $@
> +$(YUI_SYMLINK): node_modules/yui
> +	ln -sfn ../../node_modules/yui $@
>  
>  $(LP_JS_BUILD): | $(JS_BUILD_DIR)
>  	-mkdir $@
> 
> === added file 'package.json'

package.json is very hardcoded, but I managed to move it and the various other associated files into a subdirectory, which should help.

> 
> === added file 'yarn.lock'
> --- yarn.lock	1970-01-01 00:00:00 +0000
> +++ yarn.lock	2017-07-20 17:52:40 +0000
> @@ -0,0 +1,42 @@
> +# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
> +# yarn lockfile v1

It sort of makes sense if you look at it sideways because it's "locking" the results of dependency resolution so that everyone gets the same answer - but yeah, it's a pretty silly choice of name.

> +
> +
> +async@~0.2.7:
> +  version "0.2.10"
> +  resolved "https://registry.yarnpkg.com/async/-/async-0.2.10.tgz#b6bbe0b0674b9d719708ca38de8c237cb526c3d1";
> +
> +combined-stream@~0.0.4:
> +  version "0.0.7"
> +  resolved "https://registry.yarnpkg.com/combined-stream/-/combined-stream-0.0.7.tgz#0137e657baa5a7541c57ac37ac5fc07d73b4dc1f";
> +  dependencies:
> +    delayed-stream "0.0.5"
> +
> +delayed-stream@0.0.5:
> +  version "0.0.5"
> +  resolved "https://registry.yarnpkg.com/delayed-stream/-/delayed-stream-0.0.5.tgz#d4b1f43a93e8296dfe02694f4680bc37a313c73f";
> +
> +form-data@~0.0.3:
> +  version "0.0.10"
> +  resolved "https://registry.yarnpkg.com/form-data/-/form-data-0.0.10.tgz#db345a5378d86aeeb1ed5d553b869ac192d2f5ed";
> +  dependencies:
> +    async "~0.2.7"
> +    combined-stream "~0.0.4"
> +    mime "~1.2.2"
> +
> +mime@~1.2.2, mime@~1.2.7:
> +  version "1.2.11"
> +  resolved "https://registry.yarnpkg.com/mime/-/mime-1.2.11.tgz#58203eed86e3a5ef17aed2b7d9ebd47f0a60dd10";
> +
> +request@~2.14.0:
> +  version "2.14.0"
> +  resolved "https://registry.yarnpkg.com/request/-/request-2.14.0.tgz#0d8acbb0b14c1ab82e000b7d381fa8c80d1a7d88";
> +  dependencies:
> +    form-data "~0.0.3"
> +    mime "~1.2.7"

The nodejs implementations of things like Y.IO.request use the request module, so it does sort of make sense because if you're installing YUI from the npm registry then chances are you want to be able to run unit tests based on it using nodejs.  It won't actually end up in the built JS.

It's true that today this is useless to LP, but at some point it's not inconceivable that we'd want to run some tests using nodejs for speed rather than using the WebKit-based html5browser (from experience with build.snapcraft.io's entirely nodejs-based test suite, I'm not particularly concerned about compatibility problems there), and in that case we might well want request.  One way to look at it is that at the moment we only have the analogue of pagetests for JS code, and it would be useful to have something closer to unit tests too.

> +
> +yui@3.10.3:
> +  version "3.10.3"
> +  resolved "https://registry.yarnpkg.com/yui/-/yui-3.10.3.tgz#35fcea1bfafc6d435d27f13621d1ae9deb1e9f85";
> +  dependencies:
> +    request "~2.14.0"


-- 
https://code.launchpad.net/~cjwatson/launchpad/yarn/+merge/327823
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.


References