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

test(init.js): to reach 100% code coverage. #85

Merged
merged 1 commit into from May 11, 2016

Conversation

mccullagh
Copy link
Contributor

closes #79

@gr2m
Copy link
Member

gr2m commented May 8, 2016

Wow, that is spot on! Thanks for the great work! Is this your first PR to an open source project?

LGTM. PIng @hoodiehq/maintainers

@mccullagh
Copy link
Contributor Author

yes - and i realized that there are some spelling mistakes ....

on: simple.stub(),
isSignedIn: simple.stub().returnWith(false)
},
store: simple.stub(),
Copy link
Member

Choose a reason for hiding this comment

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

the method we want to stub here is not hoodie.store() but hoodie.store.connect(). Sorry I missed this before, could you please change it?

@gr2m
Copy link
Member

gr2m commented May 8, 2016

i realized that there are some spelling mistakes

In the PR? Then best to fix them right away. If you found typos in other places it’d be best start separate pull requests

@mccullagh mccullagh force-pushed the feature/new_init_tests branch 2 times, most recently from c4e5d40 to ccb3bdd Compare May 8, 2016 21:49
@mccullagh
Copy link
Contributor Author

Do i need to do something?

@gr2m
Copy link
Member

gr2m commented May 11, 2016

@mccullagh no you are all good, thanks. Usually we need two people from the maintainers team to comment with "LGTM", only then then approvals/lgtm status will be green.

But as many of our maintainers are currently conferencing through Europe and this PR doesn’t touch actual code, I’ll take advantage of my admin rights and will just merge it in :) Thanks for the great work!

@gr2m gr2m merged commit f32f385 into hoodiehq:master May 11, 2016
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.

Bring code coverage to 100% (again)
2 participants