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

Add PEP8 checks to automatic builds #3

Open
AartGoossens opened this issue Mar 10, 2018 · 4 comments
Open

Add PEP8 checks to automatic builds #3

AartGoossens opened this issue Mar 10, 2018 · 4 comments

Comments

@AartGoossens
Copy link
Contributor

I made a start with this here: https://github.com/AartGoossens/sweatpy/tree/feature/isort_pylint
Any ideas on what to check are welcome (e.g. max line length).

@piotr-kubica
Copy link
Contributor

Pep8 recommends up to 100 chars pro line.

For code maintained exclusively or primarily by a team that can reach agreement on this issue, it is okay to increase the nominal line length from 80 to 100 characters

Pylint complexity checker? https://pypi.python.org/pypi/pylint-mccabe

@AartGoossens
Copy link
Contributor Author

Thanks @piotr-kubica.
Yes I agree that enforcing <100 is more reasonable today. (although I still tend to aim for <80 because I think it's still more readable)
I did not know about the McCabe complexity checker. To be honest I have not used a complexity checker like this in a development process before. How would you use it?

@piotr-kubica
Copy link
Contributor

piotr-kubica commented Mar 15, 2018

Sometimes the methods can become very long, introducing a lot of complexity and making it hard to test. The complexity checker would then give a hint for spotting such complex methods.
On start I would set the complexity (in pylint.conf) value low and see what happens. If it spots code that is fairly straightforward then I would increase the complexity number until it reaches a spot where we can say "ok it spots complex methods that are hard to understand and test and should be split into testable units".

@AartGoossens
Copy link
Contributor Author

That sounds like a very useful thing to make part of the tests. I will include it in my PR.

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

No branches or pull requests

2 participants