Skip to content

Conversation

@daprahamian
Copy link
Contributor

@daprahamian daprahamian commented Jun 28, 2018

Rewrites all the test examples to use async/await. They
are now located in a separate folder that only runs
when the node version supports async/await

Fixes NODE-1533

WIP b/c I have to add transaction tests Transaction tests are added. Would be nice to disable console.logs in there. Also, we should check that the new file locations are good for docs.

@daprahamian daprahamian requested review from kvwalker and mbroadst June 28, 2018 23:07
Rewrites all the test examples to use async/await. They
are now located in a separate folder that only runs
when the node version supports async/await

Fixes NODE-1533
@daprahamian daprahamian force-pushed the NODE-1533/rewrite-examples branch from 6bca8b0 to a7c6350 Compare June 29, 2018 19:15
@daprahamian daprahamian changed the title WIP: docs(examples): updating examples to use async/await docs(examples): updating examples to use async/await Jun 29, 2018
Copy link
Contributor

@kvwalker kvwalker left a comment

Choose a reason for hiding this comment

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

lgtm!

Copy link
Member

@mbroadst mbroadst left a comment

Choose a reason for hiding this comment

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

LGTM, with a general nitpick that the headers for individual cases are inconsistent in capitalization

collection = undefined;
});

it('CausalConsistency', {
Copy link
Member

Choose a reason for hiding this comment

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

seems a little inconsistent (pun intended), maybe it('supports causal consistency', ...) ?

@mbroadst
Copy link
Member

mbroadst commented Jul 2, 2018

also, semver appears to be needed for node < 6, in the environment file.

@daprahamian daprahamian merged commit 61c07fd into master Jul 3, 2018
@daprahamian daprahamian deleted the NODE-1533/rewrite-examples branch July 3, 2018 18:28
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.

4 participants