Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update mocha, karma, and all related deps to their latest versions #12499

Merged
merged 4 commits into from Dec 18, 2017
Merged

Update mocha, karma, and all related deps to their latest versions #12499

merged 4 commits into from Dec 18, 2017

Conversation

rsimha
Copy link
Contributor

@rsimha rsimha commented Dec 18, 2017

This PR does the following:

List of packages updated:

mocha
karma
karma-babel-preprocessor
karma-browserify
karma-chai
karma-chrome-launcher
karma-coverage
karma-edge-launcher
karma-firefox-launcher
karma-fixture
karma-html2js-preprocessor
karma-ie-launcher
karma-mocha
karma-mocha-reporter
karma-safari-launcher
karma-sauce-launcher
karma-simple-reporter
karma-sinon-chai
karma-super-dots-reporter

Partial fix for #12181

@rsimha
Copy link
Contributor Author

rsimha commented Dec 18, 2017

/to @erwinmombay @choumx

@ampprojectbot ampprojectbot added this to the Pending Triage milestone Dec 18, 2017
@rsimha rsimha changed the title Update mocha, karma, and all their dependencies to the latest versions Update mocha, karma, and all related deps to their latest versions Dec 18, 2017
@rsimha
Copy link
Contributor Author

rsimha commented Dec 18, 2017

/cc @dvoytenko @aghassemi @lannka

return ampAd.implementation_.upgradeCallback().then(() => {
done();
});
return ampAd.implementation_.upgradeCallback();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So is the new best practice just to return Promises and no longer to use done? Or does it not matter so long as it's consistent?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh nvm. It doesn't matter so long as you're consistent. This change is just because this throws an exception now. Sorry--should have clicked through to the attached links.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep. If you're returning a Promise, the call to done() used to be ignored, but it now flags an error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants