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

Add Unit Tests for Adding Contacts #476

Merged
merged 6 commits into from Dec 29, 2017

Conversation

Projects
None yet
3 participants
@ckhatri
Contributor

ckhatri commented Dec 19, 2017

Posting this here even though it isn't done because I wanted to get some feedback to see if I was doing it right!

@j-a-m-l @perryhoffman (saw your unit tests from earlier)

@perryhoffman

Hey @ckhatri, looking good! Definitely on the right track, I left some suggestions in the comments. Reach out if any of them are unclear 👍 .

@ckhatri ckhatri changed the title from [WIP] Add Contact Unit Tests to Add Unit Tests for Adding Contacts Dec 20, 2017

@j-a-m-l j-a-m-l merged commit 85e8c41 into ArkEcosystem:master Dec 29, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@j-a-m-l

This comment has been minimized.

Show comment
Hide comment
@j-a-m-l

j-a-m-l Dec 29, 2017

Member

+5

Good work, although I've made some changes in #493 to be more comprehensible and maintain a similar style (using expect instead of sinon.assert). Please, @ckhatri have them in mind for your next contributions.

Member

j-a-m-l commented Dec 29, 2017

+5

Good work, although I've made some changes in #493 to be more comprehensible and maintain a similar style (using expect instead of sinon.assert). Please, @ckhatri have them in mind for your next contributions.

@ckhatri

This comment has been minimized.

Show comment
Hide comment
@ckhatri

ckhatri Dec 29, 2017

Contributor

@j-a-m-l gotcha!

Contributor

ckhatri commented Dec 29, 2017

@j-a-m-l gotcha!

@ckhatri ckhatri deleted the ckhatri:addContact_unit_tests branch Dec 29, 2017

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