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

Python style checking #1959

Closed
NuvandaPV opened this issue Dec 10, 2019 · 5 comments
Closed

Python style checking #1959

NuvandaPV opened this issue Dec 10, 2019 · 5 comments

Comments

@NuvandaPV
Copy link
Contributor

@NuvandaPV NuvandaPV commented Dec 10, 2019

Code style problems are frequently overlooked in our current code reviews. It would be nice to have a GitHub-plugin, that looks at any new code and warns about potential style problems. Especially for Python this should be an easy win, since there exists a set of well-known style-conventions as PEP-0008, that catch most style issues in Python code.

@NuvandaPV

This comment has been minimized.

Copy link
Contributor Author

@NuvandaPV NuvandaPV commented Dec 10, 2019

I have already looked into this last month, but only got around to opening an issue just now. I have found a GitHub-plugin called pep8speaks that seems to do exactly what we want (and nothing more), while requiring zero configuration. Here is a test I did in my local fork: NuvandaPV#2

As you can see, it will point out any style issues (but only in the lines the PR actually changes), but without doing anything invasive like blocking the merge. If there is no new style issues to be commented on, it keeps completely quiet.

@cornelinux @plettich what do you think, should we add something like this to the repo? Do you have suggestions for similar plugins?

@plettich

This comment has been minimized.

Copy link
Member

@plettich plettich commented Dec 11, 2019

I do like the fact that it only looks at changed code since there are a few PEP8 issues in the code. Can we also configure it to ignore some issues (like the line length where 79 chars is to short in my opinion)?

@cornelinux

This comment has been minimized.

Copy link
Member

@cornelinux cornelinux commented Dec 11, 2019

Code style problems are frequently overlooked in our current code reviews. It would be nice to have a GitHub-plugin, that looks at any new code and warns about potential style problems. Especially for Python this should be an easy win, since there exists a set of well-known style-conventions as PEP-0008, that catch most style issues in Python code.

How do you get this idea? The project imho had a very good and consistent code style. Do you have some examples?
Especially the mentioned example is imho not a problem in itself but a point where we need to find a consense. The old version was the correct style (line length limit) which was only lately changed.
But still a line length limit makes a lot of sense, maybe not for programming but especially for reviewing.
So basically I did not see the necessity for this issue!

@cornelinux

This comment has been minimized.

Copy link
Member

@cornelinux cornelinux commented Dec 12, 2019

I talked to @NuvandaPV about the code we would cover at first like too many blanks, wrong semi colons and so on. Sounds sensible to me. @plettich and you are right, we should ignore the line length, since we are not using ttys anymore ;-)
@NuvandaPV can you please suggest a config file? I will add the pep8speaks to the project.

... installed it. So lets add a sensible config file to exclude e.g. the line width-check.

@cornelinux

This comment has been minimized.

Copy link
Member

@cornelinux cornelinux commented Dec 12, 2019

Exclude

E501 : line to long warning or set it to 120

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.