Skip to content
This repository has been archived by the owner on Oct 23, 2024. It is now read-only.

refactor: remove async module #100

Merged
merged 7 commits into from
Jan 22, 2019

Conversation

AVaksman
Copy link
Contributor

@AVaksman AVaksman commented Dec 18, 2018

Towards googleapis/google-cloud-node/issues/2883

  • Tests and linter pass
  • Eliminates usage of async and pify modules.
  • Uses chai for asserts.
  • As per Remove usage of promisifyAll google-cloud-node#2887 discussion refactores library methods to support promises as async first with callbackification support rather than using promisifyAll.
  • Updates samples to promote async/await usage.
  • Update main system tests to test async/await and callback use of API methods.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Dec 18, 2018
@AVaksman AVaksman changed the title Refactor: remove async module (WIP)Refactor: remove async module Dec 19, 2018
Update samples to use async/await.
Uptate main system tests to use async/await and callback versions.
@AVaksman AVaksman force-pushed the remove-async-module branch from 7fc4f7a to 6ad4594 Compare December 20, 2018 18:54
@AVaksman AVaksman changed the title (WIP)Refactor: remove async module Refactor: remove async module Dec 20, 2018
@ajaaym ajaaym added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Dec 27, 2018
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Dec 27, 2018
@JustinBeckwith JustinBeckwith changed the title Refactor: remove async module refactor: remove async module Jan 15, 2019
package.json Outdated Show resolved Hide resolved
samples/fromProject.js Outdated Show resolved Hide resolved
samples/latestSpecificOS.js Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@JustinBeckwith JustinBeckwith left a comment

Choose a reason for hiding this comment

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

This is just a little hard to review, but if the system and sample tests all pass, LGTM :)

const images = is as ImageMap;
assert.ifError(err);
it('should get only the latest image from every OS', async () => {
try {
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to do the try/catch here, it'll just fail if it throws :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True, thanks, done.

});
assert(image.selfLink.indexOf(osName) > -1);
} catch (err) {
assert.ifError(err);
Copy link
Contributor

Choose a reason for hiding this comment

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

same here, you can just drop this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@JustinBeckwith JustinBeckwith added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jan 22, 2019
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jan 22, 2019
@JustinBeckwith JustinBeckwith merged commit 26c87c3 into googleapis:master Jan 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants