upgrade generators to use the latest yeoman-generator by jannyHou · Pull Request #363 · strongloop/generator-loopback

jannyHou · 2018-07-30T21:58:49Z

Review

Appreciate all your patient on reviewing the huge code change :)

~~Most of test cases pass except 4-5 to investigate(skipped at this moment), since the code change is huge I put the PR in review and am fixing the skipped ones at the same time.~~
There are 14 generators upgraded, I break down them into multiple commits, roughly 1 commit updates 1 generator.
Some commits may contain comment/debug/console.log, while they are removed in the next commits.
All the linting errors are fixed in the last commit.

Description

Here is a list of items to do/finished:

Related issues

connect to #355

Checklist

New tests added or existing tests modified to cover all changes
Code conforms with the style
guide

bajtos · 2018-07-31T15:11:25Z

    }]
+   },
+   "parserOptions": {
+      "ecmaVersion": 8


We cannot use async functions in generator-loopback yet, we need to keep support for Node.js 6.x.

Please revert this change.

That's great. I was going to update all the this.async() places to use async await.
Reverting it.

I like the idea of getting rid of this.async() - if you like to do that then I fully support you in that change!

The trick is to use Promise APIs instead of async functions.

For example:

async function foo() { await bar(); return 'result'; }

needs to be written in Promise-only styler as follows:

function foo() { return bar .then(() => 'result'); }

bajtos · 2018-07-31T15:11:36Z

+  // }

-  loadModels: actions.loadModels,
+  async ACLloadProject() {


Ditto, we cannot use async functions in generator-loopback yet.

jannyHou · 2018-08-01T19:48:01Z

 // Export it for strong-cli to use
 module.exports._package = pkg.name + ': ' + pkg.version;
 module.exports._yeoman = yeoman;
+module.exports._yeomanEnv = require('yeoman-environment');


This is related to strongloop and loopback-cli

jannyHou · 2018-08-01T20:28:52Z

 var bluemix = require('../bluemix/helpers');
 var fs = require('fs');
-var actions = require('../lib/actions');
+// var actions = require('../lib/actions');


In the first commit, please DO NOT review the change in file app/index.js.

I have to update it to test the acl generator from cli, while the app generator is not finished.

bajtos · 2018-08-02T14:00:34Z


 // Export it for strong-cli to use
 module.exports._package = pkg.name + ': ' + pkg.version;
 module.exports._yeoman = yeoman;


Do we still need to export yeoman? Isn't _yeomanEnv all that loopback-cli needs?

jannyHou · 2018-08-02T18:30:25Z

FYI yeoman-generator has an additional default option called --force-install, I feel it's ok to have it for each generator so I didn't remove it.

dhmlau · 2018-08-21T00:53:19Z

    }]
-   }
+  }
 }


pls add a new line

dhmlau · 2018-08-21T00:53:52Z

+  }
+
+  loadProject() {
+    debug('loading project...');


do we need those debug messages?

@dhmlau I think the progress info like ^ is fine, since it doesn't contain any variable.
I will double check all the places that prints variables....I don't see any sensitive info in the generator, while some probably prints long nested object. Will remove them.

ok. I have this question because it may not seem useful in the future for debugging purpose. If you think it's needed, i'm ok to keep it. thanks.

dhmlau · 2018-08-21T00:55:59Z


-    this.argument(g.f('name'), {
-      desc: g.f('Name of the application to scaffold.'),
+    this.argument('name', {


Need to keep g.f('name'), otherwise this issue #377 will appear again

Good catch.

dhmlau · 2018-08-21T00:58:24Z

+
+  bluemix() {
+    if (this.options.bluemix) {
+      this.log('\nBluemix configuration:');


I know this is out of scope for the task. Tthe string needs to be translated but can do it in another PR. i.e.
this.log(g.f('\nBluemix configuration:'));

no problem :)

dhmlau · 2018-08-21T01:00:29Z

+    this.login.apply(this);
+  };
+
+  BMpromptServiceName() {


nitpick: would it be better to have the function name as BMPromptServiceName instead of BMpromptServiceName?
Same for the functions below.

good catch.

dhmlau · 2018-08-21T01:01:12Z


-    this.argument(g.f('name'), {
-      desc: g.f('Name of the datasource to create.'),
+    this.argument('name', {


Need to keep g.f('name') instead of 'name'

dhmlau · 2018-08-21T01:03:58Z

-      ygAssert.file('./server/datasources.bluemix.js');
-      done();
-    });
+  it('should generate datasources.bluemix.js', function() {


we don't need to pass done in the parameter or call done() inside?

@dhmlau ah, don't need to. same as I put in comment loopbackio/loopback-datasource-juggler#1616 (comment), mocha handles it if a Promise returned.

dhmlau · 2018-08-21T01:06:51Z

@@ -0,0 +1,391 @@
+// Copyright IBM Corp. 2014,2016. All Rights Reserved.


the file name is called z-app.test.js. Is it possible to rename the file name to fit what it's supposed to do?

@dhmlau run the app.test.js first causes other tests fail, unfortunately I couldn't figure it out within one day so I prefer to leave it out of the scope of this PR :( I can create an issue to fix it afterwards.

i see. so the tests in this file is only a temp place to know what tests we're skipping now? i'm good with a follow up PR to handle the rest.

I am not happy about moving renaming this test file from app.test.js to z-app.test.js and back, because it prevents git from rendering a useful diff showing the changes made alongside the transfer.

If it's too difficult to fix these test properly, then I am proposing the following workaround:

In this pull request, keep the file name as app.test.js and mark the tests as skipped via describe.skip at top-level.

In the next pull request, rename the file to z-app.test.js and remove .skip.

Or maybe do both changes in the same pull request, but split them into two commits.

Thoughts?

Also BTW, the test name z-app.test.js does not make this test file the last one, there is still test/zosconnectee.test.js that's run after this one.

@bajtos True the code compare is pretty bad :( skipping it makes sense to me 👌

shimks

Good work, as the Japanese would say 'Otsukaresama'.

Has linting been run btw? I don't know whether there are going to be any unused variables lying around. I'd also run the thing through prettier

shimks · 2018-08-22T16:12:37Z

+  }

-  generateYoRc: function() {
+  commit() {


Could you please document what the point of this function is?

shimks · 2018-08-22T16:13:29Z

 // Export it for strong-cli to use
 module.exports._package = pkg.name + ': ' + pkg.version;
-module.exports._yeoman = yeoman;
+// module.exports._yeoman = yeoman;


Should this be uncommented or removed?

shimks · 2018-08-22T18:26:51Z

+ * @param {String} destination Destination file to write to. Relative to this.destinationRoot()
+ */
+
+    copy(source, dest) {


I've noticed that this function and the ones below aren't in the original file. When are these used?

@shimks good question. Let me elaborate this breaking change between yeoman-generator@0.x and yeoman-generator@3.x:

0.x has a function directory that copies the files from source to destination for you, see code here. While 3.x doesn't have it and leaves users to do the file moving.
That's why I added our implementation of directory in the actions.js file at the first place, then added the other nested functions it calls:
directory --> copy --> template

Should we write tests for these functions or are they already tested as part of the generator specific tests?

@b-admike We have tests for helper functions but not actions. Testing actions require complicated setup for generator and sandbox.
And honestly I would prefer to not add such overhead effort in this PR :-p
Also, every generator requires file moving, so we have plenty acceptance tests to prove those actions work well :)

Sounds good to me!

shimks · 2018-08-22T18:48:29Z

-  end: {
-    requireDriver: function() {
-      if (this.skip) return;
+  requireDriver() {


The previous file had the requireDriver function nested under an end object. Is this change ok?

I think end is a phase that version 0.x allows multiple functions to be executed. So I just refactor it to be a function that gets called in the original sequence. And I don't see a difference from the old generator :-p

shimks · 2018-08-22T18:49:06Z

    "loopback-soap": "^1.0.0",
    "loopback-swagger": "^5.4.0",
    "loopback-workspace": "^4.0.0",
+    "mem-fs-editor": "^5.1.0",


unused dependency

shimks · 2018-08-22T19:12:45Z


+  // Jannyhou: to be fixed!
  describe('prints right help message for each generator', function() {
+    // var CMD_NAMES = ['acl'];


This comment is not needed

shimks · 2018-08-22T19:13:04Z

        var helpFilePath = '../fixtures/help-texts/' + helpFileName;
        var output = fs.readFileSync(helpFilePath, 'utf8');
-        assert.equal(output, helpText);
+        assert.equal(helpText, output);


The arguments are switched

@shimks Yep, the old code was wrong. Read the error message you will see, the first argument is expected result, while the second one is the actual result.

shimks · 2018-08-22T19:21:47Z

+    });
+
+    it('should set dataSource to db if it exists', function() {
+      return new Promise(function(resolve, reject) {


I think we can just return wsModels.DataSourceDefinition.create; I'm assuming the method has promise support

Assuming loopback-workspace is depending on LB 3.x internally (and not on LB 2.x), then yes, methods like create should support promises out of the box.

shimks · 2018-08-22T19:23:50Z

    });

-    describe('with --bluemix', function() {
+    describe.skip('with --bluemix', function() {


From what I remember, this test might supposed to fail when run locally. Try getting rid of the skip and see if the test passes in the CI

oh...bad news. I just realized I only fixed the composed bluemix with app and datasource, while there is one with model. Sigh.
I am fixing them ^

shimks · 2018-08-22T19:25:36Z

      });
+  });
+
+  // requires generator-yeoman v0.17


Can we get rid of these comments now?

virkt25

Amazing work @jannyHou !! Everything looks good to me, just some nitpicks ... I tried pointing them out but figured you can spot them yourselves ... if not let me know and I can point out all the other spots. (; issue and self and some alignment issues for comments).

virkt25 · 2018-08-22T17:02:09Z

    msgs.push(list.map(it => '  ' + it).join('\n'));
    return msgs.join('') + '\n';
-  },
+  };


nitpick: Don't need ;

virkt25 · 2018-08-22T19:26:59Z

-      type: 'confirm',
-      default: true,
-      when: !this.bluemix,
+      var done = this.async();


Not needed.

@virkt25 It's get called on Line 173

virkt25 · 2018-08-22T19:31:58Z

+  help() {
    return helpText.customHelp(this, 'loopback_bluemix_usage.txt');
-  },
+  };


don't need the ;

virkt25 · 2018-08-22T19:33:59Z

+  help() {
    return helpText.customHelp(this, 'loopback_boot-script_usage.txt');
-  },
+  };


You shouldn't need the ;

virkt25 · 2018-08-22T19:34:04Z

      done();
    }.bind(this));
-  },
+  };


You shouldn't need the ;

virkt25 · 2018-08-22T19:47:25Z

+        },
+      ]).then(function(answers) {
+        var dir = answers.dir;
+        self.dir = dir;


Should be able to do this.dir as this function is bound to this.

virkt25 · 2018-08-22T19:47:53Z

+    };

-/**
+    /**


virkt25 · 2018-08-22T19:50:55Z

+      });
+    };
+
+    /**


virkt25 · 2018-08-22T19:51:22Z

-/**
- * Modify the list of datasources created by {@link loadDataSources}
- * and append an item for `null` datasource.
+    /**


virkt25 · 2018-08-22T19:51:39Z

+        data || this,
+        options
+      );
+      // memfs.create().copyTpl(


this can be removed.

bajtos

I am so glad to see all generators updated to the new yeoman version. Thank you @jannyHou for this gigantic effort.

I have few comments to discuss and address, see below.

bajtos · 2018-08-23T12:35:43Z

@@ -65,11 +67,12 @@ module.exports = yeoman.Base.extend({
      this.option('version', {
        desc: g.f('Display version information'),


I noticed the first argument, name, is using description property name; while all other options (not arguments!) are still using desc property name. Could you please check which form is the right one and ideally unify the implementation to use the same property name everywhere (if possible)?

@bajtos either of them works, see options in yeoman-generator and arguments in yeoman-generator

And our help message generator

But I agree it's better to unify them. I will change description to desc.

bajtos · 2018-08-23T12:37:49Z

+    }
+  };
+  printNextSteps() {
+    if (!this.options.initBluemix) {


Can we use early return here to reduce the level of indentation?

if (this.options.initBluemix) return;

bajtos · 2018-08-23T12:42:38Z

      }
    }.bind(this));
-  },
+  };


I am surprised that eslint and/or prettier did not catch these extra ;. Are we missing an eslint rule in our config?

bajtos · 2018-08-23T12:45:32Z

@@ -1,391 +0,0 @@
-// Copyright IBM Corp. 2014,2016. All Rights Reserved.


+1 for finding a more robust solution.

bajtos · 2018-08-23T12:47:45Z

+  // I manually verified that by choosing "install connector",
+  // the custom module is intalled
+  // Here it fails since folder `node_modules` is not created
+  it.skip('should support custom connector', function() {


Posting a line-level comment to remind us about a test that's skipped now.

// Jannyhou: To be fixed (3)! // I manually verified that by choosing "install connector", // the custom module is intalled // Here it fails since folder `node_modules` is not created

Hmm @bajtos could you elaborate more about what does "line-level comment" mean?

Sorry for the confusion. By line-level I meant that the comment was attached to a particular source code line (test/datasource.test.js:83) and will be hidden by GitHub once you change that code; as opposed to a "regular" comment that stays in the top-level discussion & expanded forever.

Anyhow, my intention was to point out a comment you have added and which says that a skipped test needs to be fixed. IMO, something needs to be done about that comment before landing this pull request.

If you think fixing the skipped test is out of scope of this PR, then I am totally fine with that, but please reword the comment so that it does not look like something that was slipped through the review process.

For example:

// FIXME(jannyHou) This test fails because folder `node_modules` is not created. // For now, verify this functionality manually by choosing "install connector" // and checking that the custom module is installed.

bajtos · 2018-08-23T12:51:49Z

+          paths: ['/x', '/y'],
+          params: '{"z": 1}',
+        }).then(function() {
+          // eslint-disable-next-line max-len


Let's rewrite the next line so that it fits in our max line length.

const middlewareConfig = readMiddlewaresJsonSync('server'); const newSources = Object.keys(middlewareConfig.routes);

bajtos · 2018-08-23T12:52:45Z

+    });
+
+    it('should set dataSource to db if it exists', function() {
+      return new Promise(function(resolve, reject) {


Assuming loopback-workspace is depending on LB 3.x internally (and not on LB 2.x), then yes, methods like create should support promises out of the box.

bajtos · 2018-08-23T12:57:19Z

@@ -0,0 +1,391 @@
+// Copyright IBM Corp. 2014,2016. All Rights Reserved.


I am not happy about moving renaming this test file from app.test.js to z-app.test.js and back, because it prevents git from rendering a useful diff showing the changes made alongside the transfer.

If it's too difficult to fix these test properly, then I am proposing the following workaround:

In this pull request, keep the file name as app.test.js and mark the tests as skipped via describe.skip at top-level.

In the next pull request, rename the file to z-app.test.js and remove .skip.

Or maybe do both changes in the same pull request, but split them into two commits.

Thoughts?

Also BTW, the test name z-app.test.js does not make this test file the last one, there is still test/zosconnectee.test.js that's run after this one.

jannyHou · 2018-08-27T19:17:17Z

@virkt25 Thank you for pointing out those extra semicolons! I feel the eslint definitely has some problem, having the semicolon/removing it both pass npm run lint.

And according to eslint-config-loopback, the expected behaviour is ["error", "always"], which requires the semicolon per statement. What do you think?

If we decide to remove all extra semicolon, I can add rule "semi": ["error", "always"] in .eslintrc as a workaround if fixing the lint takes time.

virkt25 · 2018-08-27T19:47:33Z

I'm indifferent to the rule as long as it's applied consistently to the code base.

jannyHou · 2018-08-27T21:27:34Z

ok I figured out why eslint didn't report error for those extra semicolons.
FYI
"no-extra-semi": "error" checks the semicolon after each class method.(inside class)
"semi": ["error", "always"] only checks the semicolon after class definition.(outside class)

And removed all extra semicolons in this commit, will keep it as a separate commit when merge PR.

bajtos

Few more minor comments to address, none of them blocking this PR from being landed as far as I am concerned.

bajtos · 2018-08-28T12:34:22Z

      "ignorePattern": "^\\s*var\\s.+=\\s*(require\\s*\\()|(/)"
-    }]
+    }],
+    "no-extra-semi": "error"


Please move this configuration to https://github.com/strongloop/eslint-config-loopback so that it's eventually enabled for all LB (3.x) projects/repositories.

bajtos · 2018-08-28T12:36:43Z

    "node": ">=6"
  },
  "scripts": {
+    "lint:fix": "eslint --ignore-path .gitignore --fix .",


Have you considered re-using the cli args from lint script via npm run lint --fix?

bajtos · 2018-08-28T12:37:00Z

-            expect(modelGen.abort).to.eql(true);
-            done();
-          });
+          var ctx = helpers.run(path.join(__dirname, '../model'));


Use const, not var - see https://loopback.io/doc/en/contrib/style-guide-es6.html

bajtos · 2018-08-28T12:37:09Z

-          expect(modelGen.abort).to.eql(true);
-          done();
-        });
+        var ctx = helpers.run(path.join(__dirname, '../model'));


Use const, not var - see https://loopback.io/doc/en/contrib/style-guide-es6.html

bajtos · 2018-08-28T12:37:14Z

-            .eql('cloudant-demo-service');
-          done();
-        });
+        var ctx = helpers.run(path.join(__dirname, '../model'));


Use const, not var - see https://loopback.io/doc/en/contrib/style-guide-es6.html

bajtos · 2018-08-28T12:38:30Z

+
+        prompts.push(toolchainPrompt, dockerPrompt);
+
+        this.prompt(prompts).then(function(answers) {


A nitpick: can you rewrite this using an arrow function and remove .bind(this)?

return this.prompt(prompts).then(answers => { // ... });

b-admike · 2018-08-28T15:10:00Z

+ * @param {String} destination Destination file to write to. Relative to this.destinationRoot()
+ */
+
+    copy(source, dest) {


Should we write tests for these functions or are they already tested as part of the generator specific tests?

b-admike · 2018-08-28T15:33:55Z

+  }

-  copyFiles: function() {
+  writing() {


nitpick: can there be a better function name here?

couldn't remember why I changed the name....reverted it.

b-admike · 2018-08-28T15:37:42Z

+  // from memory to destination folder.
+  commit() {
+    var done = this.async();
+    this.fs.commit(done);


From Yeoman docs:

It is worth noting that although this.fs exposes commit, you should not call it in your generator. Yeoman calls this internally after the conflicts stage of the run loop.

http://yeoman.io/authoring/file-system.html. It's fine if we use it, but I was wondering whether there are alternatives.

@b-admike yep it's a workaround. I eventually calls it because there is no easy better solution for finishing file move before invoke the composed generator :(

raymondfeng · 2018-08-28T17:39:18Z

+    this.login.apply(this);
+  }
+
+  BMPromptServiceName() {


The method names should be camelCase, such as bmPromptServiceName.

raymondfeng · 2018-08-28T17:40:38Z

@jannyHou Thank you for the bold efforts! Mostly LGTM.

b-admike

LGTM; Great job Janny! This was a huge undertaking and I'm glad we have these tests up and running with the shiny new version of yeoman-generator 🎉

b-admike · 2018-08-28T21:05:57Z

 var fs = require('fs');
+var rimraf = require('rimraf');
+
+// Rename it to run as the last test


nitpick: can we re-phrase this statement to start like this: The test file is named app.test.js as a workaround for...?

good catch, it's fixed lol. Let me remove the comment.

virkt25 · 2018-08-29T18:06:08Z

    // See https://github.com/strongloop/generator-loopback/issues/116
-    this.invoke = require('yeoman-generator/lib/actions/invoke');
-  },
+    // this.invoke = require('yeoman-generator/lib/actions/invoke');


Can this comment be removed?

yep good catch, removed.

jannyHou requested review from bajtos, hacksparrow and rashmihunt as code owners July 30, 2018 21:58

jannyHou force-pushed the update/yeoman-generator branch from be709b0 to 379cdda Compare July 30, 2018 22:01

bajtos reviewed Jul 31, 2018

View reviewed changes

jannyHou force-pushed the update/yeoman-generator branch from 379cdda to 6d79c28 Compare August 1, 2018 19:41

jannyHou commented Aug 1, 2018

View reviewed changes

jannyHou mentioned this pull request Aug 1, 2018

feat: switch to yeoman-env strongloop/loopback-cli#71

Merged

2 tasks

jannyHou commented Aug 1, 2018

View reviewed changes

jannyHou mentioned this pull request Aug 1, 2018

Upgrade yeoman-generator to latest #355

Closed

jannyHou force-pushed the update/yeoman-generator branch from dee08a4 to 8ee9be5 Compare August 1, 2018 21:42

bajtos reviewed Aug 2, 2018

View reviewed changes

jannyHou changed the title ~~WIP: upgrade acl generator~~ WIP: upgrade generators to use the latest yeoman-generator Aug 9, 2018

jannyHou force-pushed the update/yeoman-generator branch from c522b4e to bf89bdd Compare August 16, 2018 15:14

dhmlau mentioned this pull request Aug 20, 2018

[Snyk Update] New fixes for 1 vulnerable dependency path #349

Closed

jannyHou force-pushed the update/yeoman-generator branch from 3d06c10 to df36e65 Compare August 20, 2018 15:09

jannyHou changed the title ~~WIP: upgrade generators to use the latest yeoman-generator~~ upgrade generators to use the latest yeoman-generator Aug 20, 2018

jannyHou self-assigned this Aug 20, 2018

dhmlau reviewed Aug 21, 2018

View reviewed changes

jannyHou force-pushed the update/yeoman-generator branch from e87d95f to f351d26 Compare August 21, 2018 21:23

shimks reviewed Aug 22, 2018

View reviewed changes

virkt25 reviewed Aug 22, 2018

View reviewed changes

bajtos reviewed Aug 23, 2018

View reviewed changes

jannyHou force-pushed the update/yeoman-generator branch from 3c25a00 to c327ed1 Compare August 27, 2018 18:05

jannyHou force-pushed the update/yeoman-generator branch from 69d1a70 to 3768a8f Compare August 27, 2018 19:43

jannyHou force-pushed the update/yeoman-generator branch from b1a462e to 03bdc82 Compare August 27, 2018 21:23

bajtos reviewed Aug 28, 2018

View reviewed changes

jannyHou mentioned this pull request Aug 28, 2018

[SEMVER-MAJOR] add no-extra-semi loopbackio/eslint-config-loopback#30

Merged

b-admike reviewed Aug 28, 2018

View reviewed changes

raymondfeng reviewed Aug 28, 2018

View reviewed changes

jannyHou force-pushed the update/yeoman-generator branch from f6566ae to e39f97b Compare August 28, 2018 20:37

jannyHou mentioned this pull request Aug 28, 2018

Fix the skipped test cases after upgrading to yeoman-generator@3.x #389

Closed

b-admike approved these changes Aug 28, 2018

View reviewed changes

jannyHou force-pushed the update/yeoman-generator branch from a636ce9 to 7fcd77c Compare August 29, 2018 16:34

virkt25 approved these changes Aug 29, 2018

View reviewed changes

jannyHou force-pushed the update/yeoman-generator branch from 7fcd77c to 34d58cc Compare August 29, 2018 19:00

jannyHou added 3 commits August 29, 2018 21:54

upgrade: yeoman-generator 3.x

f2d222b

lint: fix extra semi

fa817d3

fix: apply feedback

21e6135

jannyHou force-pushed the update/yeoman-generator branch from 34d58cc to 21e6135 Compare August 30, 2018 01:54

jannyHou merged commit e038bce into master Aug 30, 2018

bajtos deleted the update/yeoman-generator branch August 30, 2018 10:53

                   }]
-                 }
+                }
               }

                
                    No newline at end of file

		@@ -0,0 +1,391 @@
		// Copyright IBM Corp. 2014,2016. All Rights Reserved.

+                  };
-              /**
+                  /**

+                    });
+                  };
+                  /**

		@@ -65,11 +67,12 @@ module.exports = yeoman.Base.extend({
		this.option('version', {
		desc: g.f('Display version information'),

		@@ -1,391 +0,0 @@
		// Copyright IBM Corp. 2014,2016. All Rights Reserved.


		prompts.push(toolchainPrompt, dockerPrompt);

		this.prompt(prompts).then(function(answers) {

Uh oh!

Conversation

jannyHou commented Jul 30, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review

Description

Related issues

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jannyHou Aug 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jannyHou commented Aug 2, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jannyHou Aug 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jannyHou Aug 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jannyHou Aug 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shimks left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jannyHou Aug 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

jannyHou commented Jul 30, 2018 •

edited

Loading

jannyHou Aug 1, 2018 •

edited

Loading

jannyHou Aug 21, 2018 •

edited

Loading

jannyHou Aug 21, 2018 •

edited

Loading

jannyHou Aug 21, 2018 •

edited

Loading

jannyHou Aug 24, 2018 •

edited

Loading

jannyHou Aug 22, 2018 •

edited

Loading