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

Fixes various lint issues #58

Merged
merged 1 commit into from
Sep 5, 2017
Merged

Conversation

syssi
Copy link
Collaborator

@syssi syssi commented Sep 5, 2017

No description provided.

@coveralls
Copy link

coveralls commented Sep 5, 2017

Coverage Status

Coverage decreased (-0.02%) to 30.932% when pulling b51ff45 on syssi:feature/lint-issues into e6c49f9 on rytilahti:master.

@syssi
Copy link
Collaborator Author

syssi commented Sep 5, 2017

Are you aware about this issue:

mirobo/vacuum_cli.py:198:1: F811 redefinition of unused 'start' from line 148
mirobo/vacuum_cli.py:206:1: F811 redefinition of unused 'stop' from line 169

@rytilahti
Copy link
Owner

Yes I'm aware, but I'm not sure what causes it unfortunately.. Regarding to your patch, I don't think the pep8 line length limitation should be strictly followed and I prefer actually keep method signatures in one line if possible, so I may change those at some point in the future.

For now let's get this merged and make a new release!

@syssi
Copy link
Collaborator Author

syssi commented Sep 5, 2017

The cause is L150 and L171. vac.manual_start and vac.start has the same method name.

@rytilahti
Copy link
Owner

Ohh, true. Mmh, I'm unsure if it would be better just to ignore that specific error for those two cases; mirobo manual start_manual instead of mirobo manual start would not be a nice solution in my eyes. As this doesn't affect the functionality, I think we can just merge this PR now.

@rytilahti rytilahti merged commit fa7308d into rytilahti:master Sep 5, 2017
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.

3 participants