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

Use str.format, raise exception in name function #32

Closed
wants to merge 1 commit into from

Conversation

ThomDietrich
Copy link
Collaborator

@ThomDietrich ThomDietrich commented Jul 26, 2017

Hey,

this change tackles the eminent error as can be seen in #31 (doesn't resolve the actual issue).

  • name method now raises an exception when no data could be retrieved
  • Unfavorable line wrapping fixed
  • string building in combination with the logger was wrong. I've replaced everything with the better str.format method e.g.
OSError: [Errno Could not read data from Mi Flora sensor %s] C4:7C:8D:60:DB:A5

@ThomDietrich
Copy link
Collaborator Author

Sorry to bother you. Any issues with this PR I should be aware of? Cheers ;)

@ghost
Copy link

ghost commented Aug 6, 2017

logging should not use string.format
lines should be no longer than 79 characters.

@ghost ghost closed this Aug 6, 2017
@ThomDietrich
Copy link
Collaborator Author

logging should not use string.format

Okay, the bug still needs to be fixed somehow...

lines should be no longer than 79 characters

A rather archaic length. Nowadays 120 or 160 is reasonable. Anyhow that's up to you.

@ghost
Copy link

ghost commented Aug 6, 2017

Please create a new pull request for the fix

About line length and a lot of other style guidelines, please check the PEP8 Python style gudielines:
https://www.python.org/dev/peps/pep-0008/#maximum-line-length

ThomDietrich added a commit to ThomDietrich/miflora that referenced this pull request Aug 6, 2017
@ThomDietrich
Copy link
Collaborator Author

Done (#35)

I'm aware of PEP8. As I said, it's up to you.

@ThomDietrich ThomDietrich deleted the patch-1 branch August 10, 2017 17:28
This pull request was closed.
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.

1 participant