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

node8, const, async #17

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

samjeffress
Copy link
Contributor

No description provided.

src/index.js Outdated
@@ -78,21 +63,21 @@ function deleteStacks(stacks, dryRun) {
});
}

function listAllStacksWrapper() {
const listAllStacksWrapper = async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

If we are not awaiting anything in this block then we can ditch the Async keyword

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that was awaited in line 14, but i've removed the intermediate listAllStacksWrapper that isn't needed now

Copy link
Contributor

Choose a reason for hiding this comment

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

Unless you are awaiting within the code block you don't need the async keyword. You can still use await on line 14 since promises are awaitable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

src/index.js Outdated
return new Promise((resolve, reject) => {
return listAllStacks(null, [], resolve, reject);
});
}

function listAllStacks(token, stackArray, resolve, reject) {
const listAllStacks = async (token, stackArray, resolve, reject) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above

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

Successfully merging this pull request may close these issues.

None yet

2 participants