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.

Sign up for GitHub

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

Jump to bottom

Replace then chains with async-await #389

Merged
merged 1 commit into from
Sep 22, 2019
Merged

Replace then chains with async-await #389

merged 1 commit into from
Sep 22, 2019

Conversation

juanjoDiaz
Copy link
Collaborator

A bit cleaner

@coveralls
Copy link

coveralls commented Apr 29, 2019 •
edited
Loading

Coverage Status

Coverage remained the same at 100.0% when pulling 58f984e on juanjoDiaz:asyncify_everything into 16cc044 on zemirco:master.

@knownasilya
Copy link
Collaborator

Is this for v5? Since we'd need to drop v6 and v7.

@juanjoDiaz
Copy link
Collaborator Author

Node 6 has been EOLed so it shouldn't be used. But you are right. Maybe we should leave this for v5.

@juanjoDiaz juanjoDiaz added this to the v5 milestone Apr 29, 2019
knownasilya
knownasilya reviewed Sep 20, 2019
View reviewed changes
test/index.js Outdated
// eslint-disable-next-line no-console
}).catch(console.error);
console.error(err);
}
}
};

Promise.all([
Copy link
Collaborator

@knownasilya knownasilya Sep 20, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Could be const [jsonFixtures, ...] = await Promise.all()

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It can't because it's at the top level of the file.

To do that we would need to wrap the whole thing in a function and then invoke it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, yeah, didn't see that with github's hiding of the unedited part 👍

@juanjoDiaz
Copy link
Collaborator Author

@knownasilya please merge this when you have time and I'll continue closing tickets for v5.

@knownasilya knownasilya merged commit f7dd7eb into zemirco:master Sep 22, 2019
@juanjoDiaz juanjoDiaz deleted the asyncify_everything branch September 22, 2019 19:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@knownasilya knownasilya knownasilya left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v5
Development

Successfully merging this pull request may close these issues.
3 participants
@juanjoDiaz @coveralls @knownasilya