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

Switch all mock imports to unittest.mock imports #2274

Merged
merged 3 commits into from Aug 27, 2019

Conversation

@mathmauney
Copy link
Contributor

commented Aug 26, 2019

Description

(Description of what the PR does, such as fixes # {issue number})
Takes care of #2261 by switching all mock imports. I've tested on python 3.4.8 and 3.6.7.

How to test

(Description of how to validate or test this PR)
Run start-mycroft.sh unittest, though Travis should take care of this.

Contributor license agreement signed?

CLA [yes] (Whether you have signed a CLA - Contributor Licensing Agreement

@pep8speaks

This comment has been minimized.

Copy link

commented Aug 26, 2019

Hello @mathmauney! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2019-08-27 09:00:40 UTC
@devs-mycroft

This comment has been minimized.

Copy link
Collaborator

commented Aug 26, 2019

Hello, @mathmauney, thank you for helping with the Mycroft project! We welcome everyone
into the community and greatly appreciate your help as we work to build an AI
for Everyone.

To protect yourself, the project, and users of Mycroft technologies we require
a Contributor Licensing Agreement (CLA) before accepting any code
contribution. This agreement makes it crystal clear that along with your
code you are offering a license to use it within the confines of this project.
You retain ownership of the code, this is just a license.

Please visit https://mycroft.ai/cla to initiate this one-time signing. Thank
you!

@forslund

This comment has been minimized.

Copy link
Member

commented Aug 26, 2019

Thanks @mathmauney, looks really good! Seems there are a couple of pep-8 issues still lingering. Could you look at those as well?

Also could remove the mock dependency from test-requirements.txt?

@devs-mycroft devs-mycroft added CLA: Yes and removed CLA: Needed labels Aug 26, 2019
@mathmauney

This comment has been minimized.

Copy link
Contributor Author

commented Aug 26, 2019

Done and done.

@forslund forslund force-pushed the mathmauney:refactor/issue-2261 branch from c038a68 to 6bca152 Aug 27, 2019
@forslund forslund force-pushed the mathmauney:refactor/issue-2261 branch from b6ec430 to 21ecf79 Aug 27, 2019
@forslund

This comment has been minimized.

Copy link
Member

commented Aug 27, 2019

I've rebased the branch and squashed the PEP8 commits. Looks very good, thank you for fixing this! Merging

@forslund forslund merged commit 2ddb230 into MycroftAI:dev Aug 27, 2019
3 checks passed
3 checks passed
:-) Build finished.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.003%) to 52.986%
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.