search

basnijholt / miflora

☘️🌡🌼🥀🏡 Mi Flora Plant sensor Python package
MIT License
363 stars 98 forks source link

Add pylint and flake8 to tox.ini and cleanup the code accordingly #47

Closed ChristianKuehnel closed 6 years ago

ChristianKuehnel commented 6 years ago

There is no real coding convention in place at the moment. It would be a good idea to add pylint and flake8 to the tox.ini and clean up the clode accordingly.

This might also mean that we need to costomize the coding conventions.

ghost commented 6 years ago

I am a newbie however I would like to work on this. Can somebody please assign me this issue??

ChristianKuehnel commented 6 years ago

That was fast, I posted this just a few minutes ago! :smile:

@ydjain sorry, I seem to be unable to assign the issue to you in github, no idea why. Feel free to work on this any way.

If you need some hints on using pylint and flake8, you can have a look at the configuration beeing used in https://github.com/home-assistant/home-assistant/blob/dev/tox.ini

ThomDietrich commented 6 years ago

Issues can only be assigned to team members. That's always a nuisance but don't worry. You've reserved the issue with your message. Happy to see others offering help!!

I think this issue is quite important and a good task for someone new. @ChristianKuehnel how do you feel about changing the line length to 120? I believe 80 is a relic from the old times and line-wraps are worse for comprehension than longer ones. 120 is a good middle path and e.g. used by GitHub in the diff view.

ghost commented 6 years ago

I have created a PR for this issue...Have ignored all the warnings emitted by flake8 Max line length parameter has been ignored temporarily...Hope this closes the issue

ChristianKuehnel commented 6 years ago

Found your PR: https://github.com/open-homeautomation/miflora/pull/53 :smile:

Why did you disable the warnings? From my point of view the benefit of flake8 is to get these warnings so that we can improve the code.

Did you also have a look at pylint?

ChristianKuehnel commented 6 years ago

@ThomDietrich yes, I agree 120 characters of line length would be a good idea for python code.

ChristianKuehnel commented 6 years ago

@ydjain Thank you for PR with the tox configuration and the cleanup of the code! :+1: :smile:

Would you also be interested in fixing the issues reported by pylint?

ghost commented 6 years ago

Sure...but I wont be able to address them until next week...will that be okay? Sorry for the delay.

ChristianKuehnel commented 6 years ago

Hi @ydjain are you still interested in fixing the remaining issues? If not I would have a look in the near future...

ChristianKuehnel commented 6 years ago

I just fixed the pylint warnings: https://github.com/open-homeautomation/miflora/pull/74