feat: web-ext automatically checks for updates #676

shubheksha · 2016-12-15T18:13:33Z

Fixes #142

coveralls · 2016-12-15T18:33:27Z

Coverage remained the same at 100.0% when pulling 268cdf8 on shubheksha:fix/142 into 9643db5 on mozilla:master.

rpl · 2016-12-15T19:28:31Z

package.json

@@ -59,10 +59,12 @@
    "node-firefox-connect": "1.2.0",
    "regenerator-runtime": "0.10.1",
    "parse-json": "2.2.0",
+    "regenerator-runtime": "0.10.0",


this line can be removed (regenerator-runtime is already in the dependencies at line 60)

rpl · 2016-12-15T19:30:49Z

src/program.js

+        version: defaultVersionGetter(absolutePackageDir),
+      };
+
+      updateNotifier({


we should probably move this before the await runCommand(argv); that is at line 112 (so that we check the version before running the actual command)

rpl · 2016-12-15T19:31:48Z

src/program.js

@@ -109,6 +110,16 @@ export class Program {
        throw new UsageError(`Unknown command: ${cmd}`);
      }
      await runCommand(argv);
+
+      let pkg = {


pkg is never re-assigned again, and so we can use const instead of let

kumar303

Breaking it into a utility function is definitely the way to go. I added some pointers about how to get started on the tests. Let me know if you have questions.

kumar303 · 2016-12-16T17:08:57Z

src/util/updates.js

@@ -0,0 +1,25 @@
+/* @flow */
+import updateNotifier from 'update-notifier';


The key to testing this function is to stub out the updateNotifier since we can trust it works as documented. The goal of the tests will be to make sure that web-ext is configuring the notifier correctly.

First, import the original package as a default that can be customized:

import defaultUpdateNotifier from 'update-notifier';

Next, use dependency injection so that it can be replaced:

export function checkForAutomaticUpdates( { name, version, updateCheckInterval, updateNotifier = defaultUpdateNotifier, }: checkForAutomaticUpdatesParams ) { // ... updateNotifier(); }

This allows you to test it with a stub:

const updateNotifier = sinon.stub(); checkForAutomaticUpdates({name, version, updateNotifier}); assert.equal(updateNotifier.firstCall.args[0].pkg.name, 'web-ext');

kumar303 · 2016-12-16T17:17:52Z

src/program.js


  constructor(
    argv: ?Array<string>,
-    {absolutePackageDir = process.cwd()}: {absolutePackageDir?: string} = {}
+    {
+      absolutePackageDir = process.cwd(),


similarly, this module / class should be set up for dependency injection so that you can replace the update utility with a stub for testing:

import {checkForAutomaticUpdates as defaultUpdateChecker} from './util/updates'; class Program { constructor( argv: ?Array<string>, { absolutePackageDir?: string, checkForAutomaticUpdates = defaultUpdateChecker, } ) { // ... this.checkForUpdates = checkForAutomaticUpdates; } }

The test can then use a stub:

const checkForAutomaticUpdates = sinon.stub(); const program = new Program(['run'], {checkForAutomaticUpdates}); await program.execute(); assert.equal(checkForAutomaticUpdates.firstCall.args[0].version, '1.0.0');

kumar303 · 2016-12-16T17:22:49Z

src/program.js

+
+      this.checkForUpdates ({
+        name: 'web-ext',
+        version: defaultVersionGetter(absolutePackageDir),


this function already has a getVersion parameter which can be replaced by a stub for testing. You can use it like checkForUpdates({version: getVersion(absolutePackageDir)})

coveralls · 2016-12-19T20:06:44Z

Coverage remained the same at 100.0% when pulling 1f66b96 on shubheksha:fix/142 into 9643db5 on mozilla:master.

coveralls · 2016-12-19T20:37:53Z

Coverage remained the same at 100.0% when pulling 305f83f on shubheksha:fix/142 into 9643db5 on mozilla:master.

coveralls · 2016-12-19T20:37:53Z

Coverage remained the same at 100.0% when pulling 305f83f on shubheksha:fix/142 into 9643db5 on mozilla:master.

kumar303

Looking good! I requested some cleanup.

I think we also need a --disable-update-check option but I suggest filing that as a separate issue and doing it in a different patch.

kumar303 · 2016-12-19T22:35:05Z

src/util/updates.js

+  updateNotifier: typeof defaultUpdateNotifier,
+};
+
+export function checkForAutomaticUpdates({


The update would technically need to be applied manually, not automatically, so I think checkForUpdates() would be a better name.

kumar303 · 2016-12-19T22:38:28Z

src/program.js

@@ -20,10 +21,15 @@ export class Program {
  yargs: any;
  commands: { [key: string]: Function };
  shouldExitProgram: boolean;
+  checkForUpdates: typeof defaultUpdateChecker;


Looks like you removed this class var so it no longer needs to be declared

kumar303 · 2016-12-19T22:39:18Z

src/program.js

    }: Object = {}
  ): Promise<void> {

    this.shouldExitProgram = shouldExitProgram;
    this.yargs.exitProcess(this.shouldExitProgram);
+    this.checkForUpdates = checkForUpdates;


it's no longer necessary to assign the function to this because it's not used anywhere else in the class

kumar303 · 2016-12-19T22:41:32Z

src/util/updates.js

+}: checkForAutomaticUpdatesParams
+) {
+  const pkg = {
+    name: name,


Why does name need to be configurable? I think you can just hard code it as name: 'web-ext'.

kumar303 · 2016-12-19T22:42:20Z

src/util/updates.js

+) {
+  const pkg = {
+    name: name,
+    version: version,


Always use shorthand property assignment when possible, like: {version}

We should probably add the lint rule for this :D http://eslint.org/docs/rules/object-shorthand

kumar303 · 2016-12-19T23:02:11Z

tests/unit/test.program.js

+import {
+  defaultVersionGetter,
+  main,
+  Program,


Any reason why you split these into multiple lines? Since the previous statement was less than 80 characters, you don't need to split it.

I added another import, so ESLint started yelling, hence I split them up but ended up removing that import and forgot to put them back in one line.

kumar303 · 2016-12-19T23:03:58Z

tests/unit/test.program.js

@@ -32,7 +36,9 @@ describe('program.Program', () => {
    let thing = spy(() => Promise.resolve());
    let program = new Program(['thing'])
      .command('thing', 'does a thing', thing);
-    return execProgram(program)
+    return execProgram(program, {
+      checkForUpdates: spy(),


Passing this directly creates unnecessary repetition. I don't see what the problem is with adding it as a default to execProgram(). What problem were you running into?

It wasn't being passed by default in execProgram() and making some of the tests fail. Not sure why.

Maybe it wasn't defined in the right part of execProgram()? If it's not working, post a broken patch and I can take a look.

kumar303 · 2016-12-19T23:05:14Z

tests/unit/test.program.js

+    const handler = spy();
+    const checkForAutomaticUpdates = sinon.stub();
+    const program = new Program(['run'])
+    .command('run', 'some command', handler);


This should be indented two spaces to show that it's a continuation of the call up above.

kumar303 · 2016-12-19T23:07:37Z

tests/unit/test.program.js

+                      'web-ext');
+        let pkgFile = path.join(root, 'package.json');
+        return fs.readFile(pkgFile)
+          .then((pkgData) => {


You should always chain promises instead of nest them. Otherwise you might as well use callbacks :) Chaining would look like:

return execProgram() .then(() => { assert.equal(...); return fs.readFile(pkgFile); }) .then((pkgFile) => { assert.equal(...); });

kumar303 · 2016-12-19T23:09:17Z

tests/unit/test.program.js

+        return fs.readFile(pkgFile)
+          .then((pkgData) => {
+            assert.equal(checkForAutomaticUpdates.firstCall.args[0].version,
+                          JSON.parse(pkgData).version);


I like this test! Good idea.

coveralls · 2016-12-20T17:15:59Z

Coverage remained the same at 100.0% when pulling 5fae3b5 on shubheksha:fix/142 into 19c08a4 on mozilla:master.

kumar303

I mostly requested nits so this is really close!

kumar303 · 2016-12-20T17:47:22Z

src/program.js

+
+      checkForUpdates ({
+        version: getVersion(absolutePackageDir),
+        updateNotifier: defaultUpdateChecker,


why does updateNotifier need to be passed? What does it do? I'm confused.

kumar303 · 2016-12-20T17:49:27Z

src/program.js

+    {
+      absolutePackageDir = process.cwd(),
+    }: {
+      absolutePackageDir?: string,


even though there is only one param, I think it's worth moving this to a standalone type object like we do elsewhere:

type ProgramOptions = { absolutePackageDir?: string, }

kumar303 · 2016-12-20T17:52:32Z

src/util/updates.js

+export function checkForUpdates({
+  version,
+  updateNotifier = defaultUpdateNotifier,
+}: checkForUpdatesParams


nit: I think it would be easier to visualize the nesting of these statements if they were indented one more level like:

export function checkForUpdates( { version, updateNotifier = defaultUpdateNotifier, }: checkForUpdatesParams ) { ... }

kumar303 · 2016-12-20T17:53:48Z

tests/unit/test-util/test.updates.js

+describe('util/updates', () => {
+  describe('checkForUpdates()', () => {
+    it('calls the notifier with the correct parameters', () => {
+      let updateNotifierStub = sinon.spy(() => {


this should use const

kumar303 · 2016-12-20T17:56:24Z

src/util/updates.js

+type checkForUpdatesParams = {
+  version: string,
+  updateNotifier?: typeof defaultUpdateNotifier,
+};


This should use the new pipe syntax in Flow that says "only these parameters are allowed" :

type checkForUpdatesParams = {| version: string, updateNotifier?: typeof defaultUpdateNotifier, |};

gah, it's not documented yet (or I just can't find where)

kumar303 · 2016-12-20T17:58:47Z

tests/unit/test-util/test.updates.js

+      });
+
+      checkForUpdates({
+        name: 'web-ext',


this call passes a bunch of unnecessary parameters but once you update the Flow type you should get errors about them

kumar303 · 2016-12-20T18:00:13Z

tests/unit/test.program.js

+    })
+      .then(() => {
+        assert.equal(checkForAutomaticUpdates.firstCall.args[0].name,
+                      'web-ext');


nit: this should line up with the opening parenthesis

actually, you don't need to make this assertion because program is no longer responsible for passing the name of the package

kumar303 · 2016-12-20T18:01:32Z

tests/unit/test.program.js

+      })
+      .then((pkgData) => {
+        assert.equal(checkForAutomaticUpdates.firstCall.args[0].version,
+                          JSON.parse(pkgData).version);


nit: this should line up with the opening parenthesis of the assert.equal() call

coveralls · 2016-12-21T17:41:09Z

Coverage remained the same at 100.0% when pulling c12d369 on shubheksha:fix/142 into 19c08a4 on mozilla:master.

kumar303

This time I gave it a test run. Looks great! I found some things that need changing.

kumar303 · 2016-12-21T17:40:59Z

src/util/updates.js

+  {
+    version,
+    updateNotifier = defaultUpdateNotifier,
+	}: checkForUpdatesParams


nit: this indent is off

kumar303 · 2016-12-21T17:42:13Z

tests/unit/test.program.js

+    })
+      .then(() => {
+        assert.equal(checkForAutomaticUpdates.firstCall.args[0].version,
+                          defaultVersionGetter(root));


nit: this is indented too much. It should line up with the opening paren like:

assert.equal(checkForAutomaticUpdates.firstCall.args[0].version, defaultVersionGetter(root));

kumar303 · 2016-12-21T17:43:50Z

tests/unit/test-util/test.updates.js

+      assert.equal(updateNotifierStub.firstCall.args[0].pkg.name, 'web-ext');
+      assert.equal(updateNotifierStub.firstCall.args[0].pkg.version, '1.0.0');
+      assert.equal(updateNotifierStub.firstCall.args[0].updateCheckInterval,
+                    1000 * 60 * 60 * 24 * 7);


nit: indent should line up with opening paren of the call

kumar303 · 2016-12-21T17:46:05Z

tests/unit/test.program.js

+  it('checks for updates automatically', () => {
+    const root = path.join(__dirname, '..', '..');
+    const handler = spy();
+    const checkForAutomaticUpdates = sinon.stub();


I know this name is left over from before; I think you should change it to avoid confusion. Actually, if you call it checkForUpdates then you'd be able to use the object property shorthand below.

kumar303 · 2016-12-21T17:49:09Z

package.json

    "parse-json": "2.2.0",
+    "regenerator-runtime": "0.10.0",


oops, I almost missed this. Please be careful when resolving conflicts. The version number was bumped down in your patch.

I didn't touch package.json at all while resolving the conflicts. There was no conflict in this file that needed to be resolved manually. I get git did it on its own?

kumar303 · 2016-12-21T17:50:51Z

src/program.js


 const log = createLogger(__filename);
 const envPrefix = 'WEB_EXT';

+type ProgramOptions = {
+  absolutePackageDir?: string,
+}


while you're already in here fixing nits how about locking down these object properties?

type ProgramOptions = {| absolutePackageDir?: string, |}

We eventually need to do this for all object types but don't worry about locking down others that are unrelated to your patch.

kumar303 · 2016-12-21T18:15:41Z

tests/unit/test.program.js

+    })
+      .then(() => {
+        assert.equal(checkForAutomaticUpdates.firstCall.args[0].version,
+                          defaultVersionGetter(root));


I noticed some test failures locally (surprisingly not on Travis though). I'm going to suggest a few things.

Stub out getVersion for all tests that are not covering any version logic

Use a stub getVersion function in your update checker test. I'm suggesting this because the behavior of defaultVersionGetter is already covered in other tests.

Here is a patch to illustrate what I mean:

diff --git a/tests/unit/test.program.js b/tests/unit/test.program.js index 58732b8..a4eea98 100644 --- a/tests/unit/test.program.js +++ b/tests/unit/test.program.js @@ -21,6 +21,7 @@ describe('program.Program', () => { let absolutePackageDir = path.join(__dirname, '..', '..'); return program.execute( absolutePackageDir, { + getVersion: () => 'not-a-real-version', checkForUpdates: spy(), systemProcess: fakeProcess, shouldExitProgram: false, @@ -251,12 +252,14 @@ describe('program.Program', () => { const checkForAutomaticUpdates = sinon.stub(); const program = new Program(['run']) .command('run', 'some command', handler); + const getVersion = () => 'some-package-version'; return execProgram(program, { + getVersion, checkForUpdates: checkForAutomaticUpdates, }) .then(() => { assert.equal(checkForAutomaticUpdates.firstCall.args[0].version, - defaultVersionGetter(root)); + 'some-package-version'); }); }); }); @@ -266,6 +269,7 @@ describe('program.main', () => { function execProgram(argv, {projectRoot = '', ...mainOptions}: Object = {}) { const runOptions = { + getVersion: () => 'not-a-real-version', checkForUpdates: spy(), shouldExitProgram: false, systemProcess: fake(process),

kumar303 · 2016-12-21T18:30:22Z

src/program.js

@@ -113,6 +120,11 @@ export class Program {
        throw new UsageError(`Unknown command: ${cmd}`);
      }
      await runCommand(argv);
+
+      checkForUpdates ({


This needs to start before the command starts, at least it does for the run command. I'm not totally sure why, I think it's because of how run exits when Firefox quits. I think it's safe to move it up because the updater runs unobtrusively in an async child process.

diff --git a/src/program.js b/src/program.js index 1f49966..33b34da 100644 --- a/src/program.js +++ b/src/program.js @@ -119,12 +119,13 @@ export class Program { if (!runCommand) { throw new UsageError(`Unknown command: ${cmd}`); } - await runCommand(argv); checkForUpdates ({ version: getVersion(absolutePackageDir), }); + await runCommand(argv); + } catch (error) { const prefix = cmd ? `${cmd}: ` : ''; if (!(error instanceof UsageError) || argv.verbose) {

It looks nice!

kumar303 · 2016-12-21T18:40:10Z

@shubheksha oh, I forgot to mention, you need to disable the update check when in development (because it will say web-ext is always out of date). Something like this:

if (localEnv === 'production') {
  checkForUpdates({version: getVersion(absolutePackageDir)});
}

You can set up localEnv similar to how it's set up for defaultVersionGetter().

coveralls · 2016-12-21T19:05:22Z

Coverage remained the same at 100.0% when pulling 672df89 on shubheksha:fix/142 into 19c08a4 on mozilla:master.

kumar303

It's getting closer. Always make sure to test your patches manually before requesting a review. For me, I do this a bit during development and then always at the final step before requesting a review from someone.

kumar303 · 2016-12-21T21:42:22Z

tests/unit/test-util/test.updates.js

+      assert.equal(updateNotifierStub.firstCall.args[0].pkg.name, 'web-ext');
+      assert.equal(updateNotifierStub.firstCall.args[0].pkg.version, '1.0.0');
+      assert.equal(updateNotifierStub.firstCall.args[0].updateCheckInterval,
+                  1000 * 60 * 60 * 24 * 7);


Sorry if I wasn't clear about this style rule. We always align the second argument to the inside of the open parenthesis which allows the reader to easily see that the statement within parentheses continues on the next line:

assert.equal(updateNotifierStub.firstCall.args[0].updateCheckInterval, 1000 * 60 * 60 * 24 * 7);

We don't have the lint rule for this enabled because it causes some other problems.

kumar303 · 2016-12-21T21:45:16Z

tests/unit/test.program.js

+    return execProgram(program, {
+      checkForUpdates,
+      getVersion,
+      localEnv: 'production',


This needs one more test that does localEnv: 'development' and makes sure the update checker doesn't get called, like assert.equal(checkForUpdates.called, false)

kumar303 · 2016-12-21T21:50:58Z

src/program.js

-      getVersion = defaultVersionGetter, shouldExitProgram = true,
+      checkForUpdates = defaultUpdateChecker, systemProcess = process,
+      logStream = defaultLogStream, getVersion = defaultVersionGetter,
+      shouldExitProgram = true, localEnv = 'development',


This won't work in production :)

We have a magic build variable which is a bit confusing. You would use it like this:

localEnv = WEBEXT_BUILD_ENV

This will set localEnv to the value of NODE_ENV at the time that web-ext was built.

Are you testing your patch manually? Automated tests are great but you also need to test it manually. The way I was doing it was temporarily decrementing the version to 1.5.0 in package.json (just for testing), running NODE_ENV=production web-ext build to get a build, then trying web-ext run or whatever to see the update notification.

coveralls · 2016-12-22T16:00:31Z

Coverage remained the same at 100.0% when pulling 2f98093 on shubheksha:fix/142 into 19c08a4 on mozilla:master.

…a new file as a utility function

…ecking for self-updates

…sions

coveralls · 2016-12-23T17:35:52Z

Coverage remained the same at 100.0% when pulling 336d0b6 on shubheksha:fix/142 into dbdf9e2 on mozilla:master.

kumar303

I'd like to change the update interval. I requested a couple other nits too but this is really close.

kumar303 · 2016-12-23T17:40:15Z

src/util/updates.js

+
+  updateNotifier({
+    pkg,
+    updateCheckInterval: 1000 * 60 * 60 * 24 * 7, // 1 week,


I think we should make this three days

kumar303 · 2016-12-23T17:40:52Z

src/program.js

-      getVersion = defaultVersionGetter, shouldExitProgram = true,
+      checkForUpdates = defaultUpdateChecker, systemProcess = process,
+      logStream = defaultLogStream, getVersion = defaultVersionGetter,
+      shouldExitProgram = true, localEnv = WEBEXT_BUILD_ENV,


Hmm, localEnv doesn't really make sense. I'm not sure why we called the last one that. I would suggest naming this globalEnv because that's what it is!

kumar303 · 2016-12-23T17:46:09Z

tests/unit/test-util/test.updates.js

+      assert.equal(updateNotifierStub.firstCall.args[0].pkg.name, 'web-ext');
+      assert.equal(updateNotifierStub.firstCall.args[0].pkg.version, '1.0.0');
+      assert.equal(updateNotifierStub.firstCall.args[0].updateCheckInterval,
+                   1000 * 60 * 60 * 24 * 7);


I didn't mention this last review but an assertion like this isn't so helpful because when you update the interval, you have to update the test too. I would suggest a looser check, something that just makes sure the option is set and that it's a number:

assert.isNumber(updateNotifierStub.firstCall.args[0].updateCheckInterval);

kumar303 · 2016-12-23T18:03:02Z

tests/unit/test.program.js

-    const runOptions = {shouldExitProgram: false, systemProcess: fake(process)};
+    const runOptions = {
+      getVersion: () => 'not-a-real-version',
+      checkForUpdates: spy(),


Since no assertions are made in these tests, you could just assign it as a sinon.stub() which does nothing. Doesn't really matter either way though.

… days

coveralls · 2016-12-23T18:14:58Z

Coverage remained the same at 100.0% when pulling fe962f6 on shubheksha:fix/142 into dbdf9e2 on mozilla:master.

shubheksha · 2016-12-23T18:16:05Z

@kumar303, could you review this again, please?

kumar303

Nice one! Let's hope this cuts down on people submitting bug reports against old versions.

kumar303 · 2016-12-24T00:04:21Z

src/program.js

@@ -138,14 +152,14 @@ declare var WEBEXT_BUILD_ENV: string;

 //A defintion of type of argument for defaultVersionGetter
 type versionGetterOptions = {
-  localEnv?: string,
+  globalEnv?: string,


feat: web-ext automatically checks for updates (mozilla#676)

muffinresearch added the state: pull request ready label Dec 15, 2016

rpl reviewed Dec 15, 2016

View reviewed changes

shubheksha force-pushed the fix/142 branch from 268cdf8 to 1080d16 Compare December 15, 2016 19:52

kumar303 reviewed Dec 16, 2016

View reviewed changes

kumar303 self-assigned this Dec 16, 2016

kumar303 suggested changes Dec 19, 2016

View reviewed changes

shubheksha force-pushed the fix/142 branch from 305f83f to 1de06f2 Compare December 20, 2016 17:03

shubheksha mentioned this pull request Dec 20, 2016

Add an option to disable checking for updates #694

Open

kumar303 suggested changes Dec 20, 2016

View reviewed changes

kumar303 suggested changes Dec 21, 2016

View reviewed changes

rpl mentioned this pull request Dec 22, 2016

feat: show desktop notifications on addon reload #674

Merged

shubheksha added 7 commits December 23, 2016 22:49

feat: web-ext checks for updates and alerts the user to update

f3d017f

fix: saved update-notifier to package.json

40f2e64

test: [wip] check for automatic updates and alert the users

95e2921

feat: checks for updates automatically and alerts user - moved it to …

6380103

…a new file as a utility function

test: [wip] check for self-updates automatically

6f22604

test: all tests existing pass after adding the notifier for self updates

9c903f1

test: the program object correctly calls the notifier function for ch…

1316d7d

…ecking for self-updates

shubheksha added 7 commits December 23, 2016 22:55

test: refactored tests, cleaned up program.js and updates.js

0ba8c14

fix: fixed eslint errors

5973f28

fix: style nits

7602131

test: fixed test to work in both development and production environment

4088d49

fix: style nits and stubbed getVersion() for tests not related to ver…

79cf3be

…sions

fix: checking for updates is disable during development

09da85a

test: added test for disabling checking of updates during development

336d0b6

shubheksha force-pushed the fix/142 branch from 2f98093 to 336d0b6 Compare December 23, 2016 17:30

kumar303 suggested changes Dec 23, 2016

View reviewed changes

refactor: changed localEnv to globalEnv, reduced update interval to 3…

fe962f6

… days

kumar303 approved these changes Dec 24, 2016

View reviewed changes

kumar303 merged commit 31d2e46 into mozilla:master Dec 24, 2016

kumar303 removed the state: pull request ready label Dec 24, 2016

aniketkudale added a commit to aniketkudale/web-ext that referenced this pull request Dec 24, 2016

Merge pull request #2 from mozilla/master

75ce2de

feat: web-ext automatically checks for updates (mozilla#676)

rpl mentioned this pull request Oct 27, 2022

web-ext should tell users when a new version is available #2539

Open

		@@ -0,0 +1,25 @@
		/* @flow */
		import updateNotifier from 'update-notifier';

feat: web-ext automatically checks for updates #676

feat: web-ext automatically checks for updates #676

Uh oh!

Conversation

shubheksha commented Dec 15, 2016

Uh oh!

coveralls commented Dec 15, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

kumar303 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!

coveralls commented Dec 19, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coveralls commented Dec 19, 2016

Uh oh!

coveralls commented Dec 19, 2016

Uh oh!

kumar303 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!

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!

Choose a reason for hiding this comment

Uh oh!

coveralls commented Dec 20, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kumar303 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!

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!

coveralls commented Dec 15, 2016 •

edited

Loading

coveralls commented Dec 19, 2016 •

edited

Loading

coveralls commented Dec 20, 2016 •

edited

Loading

coveralls commented Dec 21, 2016 •

edited

Loading