chore(*): use Yarn for dependency management #15435

petebacondarwin · 2016-11-25T15:53:26Z

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

What is the current behavior? (You can also link to an open issue here)

What is the new behavior (if this is a feature change)?

Does this PR introduce a breaking change?

Please check if the PR fulfills these requirements

The commit message follows our guidelines: https://github.com/angular/angular.js/blob/master/CONTRIBUTING.md#commit-message-format
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

Other information:

Narretz · 2016-11-25T19:57:23Z

Node 6, here we come!
Can we have an explicite mention of the upgrade to node 6 in one of the commit messages? Is it implied by the change to yarn?

gkalpak

🎉 BTW, we also need to update docs/content/misc/contribute.ngdoc.

gkalpak · 2016-11-25T17:25:55Z

scripts/travis/build.sh

@@ -6,19 +6,19 @@ export BROWSER_STACK_ACCESS_KEY=`echo $BROWSER_STACK_ACCESS_KEY | rev`
 export SAUCE_ACCESS_KEY=`echo $SAUCE_ACCESS_KEY | rev`

 if [ "$JOB" == "ci-checks" ]; then
-  grunt ci-checks
+node_modules/.bin/grunt ci-checks


Bad indentation (here and below).

gkalpak · 2016-11-26T12:09:53Z

scripts/validate-commit-msg.js

@@ -16,7 +16,7 @@ var util = require('util');



Is this file used any more (with init-repo.sh gone)? We could as well get rid of it altogether.

gkalpak · 2016-11-26T12:15:51Z

src/ng/directive/input.js

@@ -22,10 +22,10 @@ var ISO_DATE_REGEXP = /^\d{4,}-[01]\d-[0-3]\dT[0-2]\d:[0-5]\d:[0-5]\d\.\d+(?:[+-
 //   8. Query
 //   9. Fragment
 //                 1111111111111111 222   333333    44444        555555555555555555555555    666     77777777     8888888     999


Nit: The last 5 should be a space.

gkalpak · 2016-11-26T12:22:02Z

scripts/jenkins/set-node-version.sh

+# Use version of node.js found in .nvmrc
+nvm install
+
+npm install -g yarn


Nit: This file now does more than its name suggests. Maybe rename it to something like setup-node-env?

petebacondarwin · 2016-11-26T20:59:27Z

I'll add better commit messages. I am pretty sure that yarn requires node 6+.

mgol · 2016-11-28T10:16:26Z

Gruntfile.js

@@ -322,13 +318,6 @@ module.exports = function(grunt) {
    }
  });

-  // global beforeEach task
-  if (!process.env.TRAVIS) {


Shouldn't we run yarn here?

OK, that sounds right.

mgol · 2016-11-28T10:17:19Z

changelog.js

@@ -1,208 +0,0 @@
-#!/usr/bin/env node
-


Does this removal have anything to do with switching to Yarn? Looks like a general cleanup.

Yes, you're right, I will move it into separate commits

mgol · 2016-11-28T10:20:52Z

src/ngComponentRouter/Router.js

@@ -10,11 +10,11 @@
 *
 * @installation
 *
- * Currently, the **Component Router** module must be installed via `npm`, it is not yet available
+ * Currently, the **Component Router** module must be installed via `npm`/`yarn`, it is not yet available


Unrelated, but this "yet" is a little reaching... :) I'd remove the word.

I'll do so in a separate commit :-)

mgol · 2016-11-28T10:21:53Z

package.json

@@ -9,8 +9,8 @@
    "url": "https://github.com/angular/angular.js.git"
  },
  "engines": {
-    "node": "<5",
-    "npm": "~2.5"
+    "node": "^6.9.0",


Maybe ^6.9.1? 6.9.1 was released a day after 6.9.0 and contained critical fixes.

gkalpak

Added a few more nits 😃

gkalpak · 2016-11-28T09:39:33Z

docs/content/guide/component-router.ngdoc

@@ -467,13 +467,12 @@ to display list and detail views of Heroes and Crises.

 ## Install the libraries

-It is easier to use npm to install the **Component Router** module. For this guide we will also install
-AngularJS itself via npm:
+It is easier to use [yarn](https://yarnpkg.com) to install the **Component Router** module.


Given yarn is still new, I would mention npm here too:

It is easier to use npm/yarn to install...

gkalpak · 2016-11-28T09:47:02Z

docs/content/misc/contribute.ngdoc

@@ -62,7 +69,7 @@ cd angular.js
 git remote add upstream "https://github.com/angular/angular.js.git"

 # Install node.js dependencies:
-npm install
+yarn

 # Install bower components:
 bower install


Not directly related to this PR:
This implies a global bower installation as well. This step isn't necessary though, since grunt package takes care of it (without requiring a global bower installation).

gkalpak · 2016-11-28T09:47:28Z

docs/content/misc/contribute.ngdoc

@@ -62,7 +69,7 @@ cd angular.js
 git remote add upstream "https://github.com/angular/angular.js.git"

 # Install node.js dependencies:
-npm install
+yarn


Maybe yarn install for "declarativeness".

Hmm, the Yarn docs generally recommend just yarn but OK.

gkalpak · 2016-11-28T09:49:45Z

i18n/generate.sh

@@ -3,14 +3,13 @@
 set -e

 BASE_DIR=`dirname $0`
-cd $BASE_DIR


OOC, why this change?

This reminded me that scripts like this one are dangerous unless you add set -u at the beginning. Otherwise if suddenly a variable like that gets unset and later used, you may get unexpected results.

@gkalpak the reason for the change is that yarn currently does not support being run from a subfolder. It must be run from where the package.json is held. See yarnpkg/yarn#2053

@mgol - so this is the background, right? http://www.davidpashley.com/articles/writing-robust-shell-scripts/
In this case we are using the variable directly after defining it and the file is very small, so there is little chance of not being set.
Are you concerned that this variable will leak outside of the script and be relied upon elsewhere? Is that even possible without exporting the variable?

gkalpak · 2016-11-28T09:53:05Z

scripts/jenkins/init-node.sh

+# Use version of node.js found in .nvmrc
+nvm install
+
+npm install -g yarn


No comment for this little fellow? 😞

We should install a version matching what we have in package.json's engines.yarn field.

jq is a good tool for that. You can play with it at https://jqplay.org/

Does the engines field actually have any effect?
I would have thought that calling yarn install with an invalid yarn version should cause some kind of warning or error from yarn or node or something. Otherwise, what is the point?
Although we could use engines to tell us what to install on jenkins, what about us humans who don't update their global yarn installations then the engines value changes?

Some environments, like Heroku, use engines to decide what version to use. I guess it won't happen in our situation but we should still specify the version somewhere and it doesn't hurt to do it in engines and read from there; if someone wanted to do sth on one of these engines-reading tools, they could use these settings.

As for humans, we could always add a check for an installed version and require it to be not older than whatever's in engines.yarn.

gkalpak · 2016-11-28T09:55:08Z

scripts/travis/build.sh

-  grunt tests:docs --browsers="$BROWSERS" --reporters=dots
+node_modules/.bin/grunt test:promises-aplus
+node_modules/.bin/grunt test:unit --browsers="$BROWSERS" --reporters=dots
+node_modules/.bin/grunt tests:docs --browsers="$BROWSERS" --reporters=dots


Bad indentation (here and below) 😟

gkalpak · 2016-11-28T09:56:10Z

src/ng/directive/input.js

@@ -21,11 +21,11 @@ var ISO_DATE_REGEXP = /^\d{4,}-[01]\d-[0-3]\dT[0-2]\d:[0-5]\d:[0-5]\d\.\d+(?:[+-
 //   7. Path
 //   8. Query
 //   9. Fragment
-//                 1111111111111111 222   333333    44444        555555555555555555555555    666     77777777     8888888     999
-var URL_REGEXP = /^[a-z][a-z\d.+-]*:\/*(?:[^:@]+(?::[^@]+)?@)?(?:[^\s:/?#]+|\[[a-f\d:]+\])(?::\d+)?(?:\/[^?#]*)?(?:\?[^#]*)?(?:#.*)?$/i;
+//                 1111111111111111 222   333333    44444        55555555555555555555555     666     77777777     8888888     999


The lack of named capture groups in JavaScript regular expressions is really annoying :P

gkalpak · 2016-11-28T10:39:25Z

Gruntfile.js

@@ -322,13 +318,6 @@ module.exports = function(grunt) {
    }
  });

-  // global beforeEach task
-  if (!process.env.TRAVIS) {
-    grunt.task.run('shell:npm-install');


FWIW, I think this was kind of useful...
_{(I can leave without it though.)}

gkalpak · 2016-11-28T11:00:01Z

scripts/jenkins/release.sh

@@ -38,8 +38,7 @@ function build {
  source ./init-node.sh
  cd ../..

-  yarn
-  node_modules/.bin/grunt  ci-checks package --no-color
+  grunt  ci-checks package --no-color


Two spaces between 'grunt' and 'ci' 😱

gkalpak · 2016-11-28T11:01:33Z

scripts/jenkins/build.sh

@@ -21,22 +21,21 @@ rm -f angular.js.size


 # BUILD #
-yarn
-node_modules/.bin/grunt ci-checks package --no-color
+grunt ci-checks package --no-color


Is grunt available globally?

gkalpak · 2016-11-28T11:03:51Z

Gruntfile.js

-      'npm-install': {
-        command: 'node scripts/npm/check-node-modules.js'
+      'install-node-dependencies': {
+        command: 'yarn'


Won't this take more time that simply comparing two files (s was happening before). I am not very keen on sacrificing speed for every task for this 😕

It shouldn't be a problem. The following was run on the angular.js repo with npm & yarn caches cleared:

$ time yarn --ignore-scripts yarn --ignore-scripts 28.22s user 21.07s system 125% cpu 39.178 total $ time yarn --ignore-scripts yarn --ignore-scripts 6.49s user 2.68s system 120% cpu 7.583 total $ time yarn --ignore-scripts yarn --ignore-scripts 0.87s user 0.07s system 70% cpu 1.321 total $ time yarn --ignore-scripts yarn --ignore-scripts 0.86s user 0.07s system 70% cpu 1.313 total

Right 👍

When trying this myself, I stopped after the 2nd time 😃

Could you try? It'd be good to make sure it's that quick not only on *NIX-es.

I did try of course. I didn't take your word for it 😛
It "stabilizes" at 2secs on my machine, which is OK (for me at least).

I wonder why it's not that fast on the second try, though. I'd expect it to behave identically in all situations where cache is populated and dependencies are correct so something looks wrong here.

Let's see: yarnpkg/yarn#2054

gkalpak · 2016-11-28T11:24:47Z

scripts/travis/build.sh

-node_modules/.bin/grunt tests:docs --browsers="$BROWSERS" --reporters=dots
+grunt test:promises-aplus
+grunt test:unit --browsers="$BROWSERS" --reporters=dots
+grunt tests:docs --browsers="$BROWSERS" --reporters=dots


Bad indentation 😛

mgol · 2016-11-28T11:48:35Z

scripts/jenkins/init-node.sh

+# Use version of node.js found in .nvmrc
+nvm install
+
+npm install -g yarn


We should install a version matching what we have in package.json's engines.yarn field.

gkalpak

A couple of minor comments/questions.
LGTM

gkalpak · 2016-11-28T18:06:07Z

.nvmrc

@@ -1,2 +1 @@
-4
-
+6.9.1


Isn't that too restrictive?

6.9 is the LTS version of node. There are actually only two versions node of 6.9: 6.9.0 and 6.9.1 The former has issues that need to be fixed in 6.9.1. I don't see any reason why we should let people use versions below 6.9.

Moreover nvm is not capable of parsing ^6.9.1 so we would be stuck with 6.9 or worse 6, which could allow versions that we don't want such as 6.8.x and 6.9.0.

But it means that we always need to have (and switch to) 6.9.1 locally, even when 6.9.2+ comes out 😞

gkalpak · 2016-11-28T18:20:06Z

Gruntfile.js

@@ -322,13 +333,10 @@ module.exports = function(grunt) {
    }
  });

-  // global beforeEach task


You could leave this.

OK, although I found it slightly confusing due to its similarity to testing terminology - like it was something special. How about I modify it to something more humane?

gkalpak · 2016-11-28T18:23:05Z

docs/content/misc/contribute.ngdoc

+  development web server, run tests, and generate distributable files. Depending on your system,
+  you can install Node either from source or as a pre-packaged bundle.
+
+  We recommend using [nvm](https://github.com/creationix/nvm) to manage and install Node.js,


I've been using https://github.com/coreybutler/nvm-windows for some time now and it works fine. I think it is worth mentioning it too.

gkalpak · 2016-11-28T18:24:53Z

docs/content/misc/contribute.ngdoc

+  We recommend using [nvm](https://github.com/creationix/nvm) to manage and install Node.js,
+  which makes it easy to change the version of Node.js per project.
+
+* [Yarn](https://yarnpkg.com): We use Yarn to install our Node.js module dependencies (rather than using npm).


It might be worth mentioning the installation command (npm install -g yarn).

@gkalpak - It's better to link to https://yarnpkg.com/en/docs/install. Installing Yarn via npm is not the recommended installation method and should only be used in cases where a proper package is not available for your environment.

gkalpak · 2016-11-28T18:28:19Z

package.json

@@ -9,16 +9,13 @@
    "url": "https://github.com/angular/angular.js.git"
  },
  "engines": {
-    "node": "<5",
-    "npm": "~2.5"
+    "node": "6.9.1",


Why restrict this to 6.9.1?

gkalpak · 2016-11-28T18:32:06Z

scripts/travis/build.sh

-  grunt tests:docs --browsers="$BROWSERS" --reporters=dots
+grunt test:promises-aplus
+grunt test:unit --browsers="$BROWSERS" --reporters=dots
+grunt tests:docs --browsers="$BROWSERS" --reporters=dots


Bad indentation (this one never gets old 😛 )

Just keeping you interested :-P

gkalpak · 2016-11-28T18:38:56Z

BTW, I have tested this on Windows (10) and Ubuntu (16.04) with yarn && grunt test and everything works fine.

Daniel15 · 2016-11-28T19:31:26Z

Please avoid installing Yarn via npm if possible. This is not recommended. It's significantly slower than all other methods of installing Yarn, and often results in weird issues around Yarn's dependencies.

If running on Debian or Ubuntu, you should use Yarn's Debian repository. For Windows, use Chocolatey or the installer. Otherwise, use the installation script (curl -o- -L https://yarnpkg.com/install.sh | bash) or just extract the tarball manually. Both methods (Debian package repository and installation script) allow you to specify the exact version of Yarn to install.

petebacondarwin · 2016-11-28T21:09:28Z

@Daniel15 - thanks for the feedback about installing Yarn. Do you have any input on how we could lock down the version of Yarn that we are using to install our dependencies?

Daniel15 · 2016-11-28T21:16:25Z

It depends on how you're installing Yarn. If you use the Debian repo, you can pass the version number to apt-get (eg. `apt-get install yarn=0.17.8-1`). If you use the install script, it can take a version as a command line argument (eg. `./install.sh --version 0.17.8`). Locking down the Yarn version used on CI servers is a good idea. Sent from my phone.

…

On Nov 28, 2016 1:10 PM, "Pete Bacon Darwin" ***@***.***> wrote: @Daniel15 <https://github.com/Daniel15> - thanks for the feedback about installing Yarn. Do you have any input on how we could lock down the version of Yarn that we are using to install our dependencies? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#15435 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAFnHaJUypHpVKIYIjIGTinPlg9J47Duks5rC0NEgaJpZM4K8jcz> .

mgol · 2016-11-28T21:48:20Z

scripts/travis/before_build.sh

@@ -2,6 +2,8 @@

 set -e

+yarn global add grunt-cli


Let's lock the version. ;-)

petebacondarwin · 2016-11-28T22:31:51Z

@Daniel15 - thanks. I think we are going to go with:

curl -o- -L https://raw.githubusercontent.com/yarnpkg/yarn/v0.17.9/scripts/install-latest.sh | bash -s -- --version 0.17.9

which ensures that the install script is also locked to the version being installed.

Daniel15 · 2016-11-28T23:02:11Z

@petebacondarwin - The version of install-latest.sh in the 0.17.9 tag is missing some features, such as GPG verification and the --version flag. I don't think we cherry-picked any of the changes to the install script into the stable branch, as we expect everyone to use https://yarnpkg.com/install.sh rather than directly referencing the script on GitHub. Eventually the script will move out of the Yarn repo into the releases repo, where the Debian repo is also kept.

If you like, you could just have your own small script that downloads and extracts Yarn, based off the install-latest.sh script. Or, just grab https://yarnpkg.com/install.sh and check it into your repo.

mgol · 2016-11-28T23:16:10Z

@Daniel15 Pete & me have tried using the install script but it doesn't work... I've tried downloading it & evaluating on the fly:

$ curl -o- -L https://yarnpkg.com/install.sh | bash -s -- --version 0.17.9
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100    94  100    94    0     0    205      0 --:--:-- --:--:-- --:--:--   206
100  6074  100  6074    0     0   7515      0 --:--:-- --:--:-- --:--:-- 43078
Installing Yarn!
bash: line 164: yarn: command not found

as well as downloading first & then using:

$ curl -O -L https://yarnpkg.com/install.sh
$ chmod +x install.sh
$ ./install.sh --version 0.17.9
Installing Yarn!
./install.sh: line 164: yarn: command not found

EDIT: I've tried it on ZSH which I use by default but it's the same when using Bash. This probably shouldn't matter as the script itself uses Bash.

Daniel15 · 2016-11-28T23:50:53Z

@mgol - Looks like there's a bug in the script when ~/.yarn exists but there's no Yarn version installed in it. I just filed an issue for it: yarnpkg/yarn#2061

gkalpak · 2016-11-29T12:39:12Z

scripts/travis/before_build.sh

@@ -2,7 +2,7 @@

 set -e

-yarn global add grunt-cli
+yarn global add grunt-cli@1.2.0


AFAICT, this will use the latest 1.x release. Is that intended or should we use the --exact option?

This is weird, npm in this case would install the provided version. Also, Yarn docs are contradicting each other in the very same page:

yarn add [email protected] installs a specific version of a package from the registry.

vs:

For example, yarn add [email protected] would accept version 1.9.1, but yarn add [email protected] --exact would only accept version 1.2.3.

@Daniel15 could you clarify which one is right?

Good spot!
Perhaps we should use --tilde?

No, it's CLI, we should lock everything as much as possible.

Trying this locally, it seems that it will install the specific version (i.e. --exact makes no difference).
Probably a docs issue.

gkalpak · 2016-11-29T12:39:32Z

scripts/travis/build.sh

-grunt tests:docs --browsers="$BROWSERS" --reporters=dots
+  grunt test:promises-aplus
+  grunt test:unit --browsers="$BROWSERS" --reporters=dots
+  grunt tests:docs --browsers="$BROWSERS" --reporters=dots


Good indentation! 😛

This became reduntant as of 507ee2d

We already use `git clone ... --depth=1` for the `code.angularjs.org` repository, which speeds up the cloning of the repository.

grunt is installed globally on jenkins so we can just use it directly.

If global versions of node, yarn or grunt-cli don't match what we expect then blow up.

gkalpak

LGTM

gkalpak · 2016-11-29T13:42:25Z

Gruntfile.js

+// Node.js version checks
+if (!semver.satisfies(process.version, pkg.engines.node)) {
+  reportOrFail('Invalid node version (' + process.version + '). ' +
+               'Please use a version that satisfies ' + pkg.engines.node);


Nit: Missing trailing period 😁
Nit: I would make the message a little "softer". For example, the build will (hopefully) work fine with v7+; calling it an "invalid version" sounds "strong".

You're too kind! If you feel strongly about it I will let you fix up the messaging in a future commit.

Along with the missing full stops

gkalpak · 2016-11-29T13:43:07Z

Gruntfile.js

+var currentYarnVersion = exec('yarn --version', {silent: true}).stdout.trim();
+if (!semver.satisfies(currentYarnVersion, expectedYarnVersion)) {
+  reportOrFail('Invalid yarn version (' + currentYarnVersion + '). ' +
+               'Please use a version that satisfies ' + expectedYarnVersion);


Nit: Missing trailing period.

gkalpak · 2016-11-29T13:45:35Z

Gruntfile.js

+} else {
+  if (!semver.satisfies(match[1], expectedGruntVersion)) {
+  reportOrFail('Invalid grunt-cli version (' + match[1] + '). ' +
+               'Please use a version that satisfies ' + expectedGruntVersion);


Nit: Missing trailing period.

petebacondarwin added needs: review type: chore labels Nov 25, 2016

googlebot added the cla: yes label Nov 25, 2016

petebacondarwin force-pushed the yarn branch from 705372c to aef01c0 Compare November 25, 2016 17:12

gkalpak reviewed Nov 26, 2016

View reviewed changes

mgol reviewed Nov 28, 2016

View reviewed changes

gkalpak reviewed Nov 28, 2016

View reviewed changes

mgol suggested changes Nov 28, 2016

View reviewed changes

petebacondarwin force-pushed the yarn branch from 37e79f6 to 69904ed Compare November 28, 2016 16:33

gkalpak approved these changes Nov 28, 2016

View reviewed changes

mgol suggested changes Nov 28, 2016

View reviewed changes

petebacondarwin added needs: work and removed needs: review labels Nov 28, 2016

gkalpak reviewed Nov 29, 2016

View reviewed changes

petebacondarwin force-pushed the yarn branch from c76502f to 38ce99e Compare November 29, 2016 12:44

chore(dependencies): USE YARN (and node 6)

05c3336

petebacondarwin added 14 commits November 29, 2016 13:25

docs(*): switch from npm to yarn

1dedcdf

style(*): fix no-useless-escape eslint errors

736b6c7

chore(npm-bundle-deps): remove unused script

d4863a8

This became reduntant as of 507ee2d

chore(bower): set clone depth to speed up releases

e8f9cbf

We already use `git clone ... --depth=1` for the `code.angularjs.org` repository, which speeds up the cloning of the repository.

chore(saucelabs): update sauce-connect to 4.4.1

ad4e86a

chore(validate-commit): remove redundant scripts

aadee89

docs(ComponentRouter): it is unlikely ever to appear on bower or CDN

d003ec1

chore(jenkins): remove path to grunt

f54e924

grunt is installed globally on jenkins so we can just use it directly.

chore(travis): install grunt-cli globally to simplify scripts

0990833

chore(utils): install npm-run to simplify scripts

ca139de

chore(grunt): run gulp directly rather than through npm

5dd3a35

chore(grunt): check node, yarn and grunt-cli versions

f04fcdf

If global versions of node, yarn or grunt-cli don't match what we expect then blow up.

docs(FAQ) add link to security guide

afafb7a

docs(contribute): mention nvm-windows

b77defd

petebacondarwin force-pushed the yarn branch from 192d9f1 to b77defd Compare November 29, 2016 13:26

petebacondarwin added needs: review and removed needs: work labels Nov 29, 2016

mgol approved these changes Nov 29, 2016

View reviewed changes

gkalpak approved these changes Nov 29, 2016

View reviewed changes

petebacondarwin merged commit b77defd into angular:master Nov 29, 2016

chore(*): use Yarn for dependency management #15435

chore(*): use Yarn for dependency management #15435

Conversation

petebacondarwin commented Nov 25, 2016

Narretz commented Nov 25, 2016

gkalpak left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

petebacondarwin commented Nov 26, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gkalpak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mgol Nov 28, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gkalpak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gkalpak commented Nov 28, 2016

Daniel15 commented Nov 28, 2016 • edited Loading

petebacondarwin commented Nov 28, 2016

Daniel15 commented Nov 28, 2016 via email

Choose a reason for hiding this comment

Choose a reason for hiding this comment

petebacondarwin commented Nov 28, 2016

Daniel15 commented Nov 28, 2016 • edited Loading

mgol commented Nov 28, 2016 • edited Loading

Daniel15 commented Nov 28, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gkalpak left a comment •

edited

Loading

mgol Nov 28, 2016 •

edited

Loading

Daniel15 commented Nov 28, 2016 •

edited

Loading

Daniel15 commented Nov 28, 2016 •

edited

Loading

mgol commented Nov 28, 2016 •

edited

Loading