architv / soccer-cli

:soccer: Football scores for hackers. :computer: A command line interface for all the football scores.
MIT License
1.09k stars 222 forks source link

Fix some pep8 errors #39

Closed Saturn closed 8 years ago

Saturn commented 8 years ago

Kept really long lines :smile: Add two line breaks between end of function and next line Fix trailing whitespace Fix inconsistent spacing around some = or :

Just some simple cleanup. Zero code changes.

ueg1990 commented 8 years ago

This is gr8! 1) You sure PEP8 says two line breaks between functions? I thought it was one 2) Should we decide on what is the hard limit for number of characters per line? Standard is 79 but it is not strict. Maybe hard limit of 85?

Saturn commented 8 years ago

I think it is two breaks. I hope it is because that is what I have been doing forever.

Some lines are real long and a bit difficult to deal with. Maybe aim for below 90 characters to be even more generous though.

On Sun, 13 Sep 2015 14:23 Usman Ehtesham Gul notifications@github.com wrote:

This is gr8! 1) You sure PEP8 says two line breaks between functions? I thought it was one 2) Should we decide on what is the hard limit for number of characters per line? Standard is 79 but it is not strict. Maybe hard limit of 85?

— Reply to this email directly or view it on GitHub https://github.com/architv/soccer-cli/pull/39#issuecomment-139873508.

architv commented 8 years ago

@Saturn +1. But when I run pep8 main.py I get this.

selection_017 selection_018

Even 90 would fall short.

ueg1990 commented 8 years ago

You can actually pass parameters with the pep8 command to add condtions. Here is an example of one that i used before: pep8 --exclude=LICENSE,.txt,.md,.pyc,.svn,CVS,.bzr,.hg,.git,pycache --ignore=E501

Saturn commented 8 years ago

I basically ignored all the line too long ones because I didn't want to mess about too much with the formatting.

I fixed a lot of the continuation under-indented errors though. The ones that remain are ones that would be quite awkward looking if they were to be fixed.

Some of the really long lines can be dealt with quite easily. You can see that nearly all of the offending lines are between 80 and 90 characters.

Could you pass a parameter saying you want the line length limit to be 90 or 100 instead of 80? To make it easier to read the output of the tool.

architv commented 8 years ago

I think the code looks fine now and formatting any more lines to make them less than 80 characters would be overdoing the pep8 instructions. After ignoring both line too long and continuation under-indented errors, these are the only errors left

archit@archit-Inspiron-5520:~/Documents/DjangoLabs/gswd/projects/soccer_cli/soccer$ pep8 --ignore=E501,E128 main.py 
main.py:175:13: E126 continuation line over-indented for hanging indent
main.py:193:58: W291 trailing whitespace
main.py:194:65: E231 missing whitespace after ','
main.py:199:65: E231 missing whitespace after ','
main.py:204:65: E231 missing whitespace after ','
main.py:209:65: E231 missing whitespace after ','
main.py:272:5: E731 do not assign a lambda expression, use a def
main.py:349:5: E123 closing bracket does not match indentation of opening bracket's line
main.py:354:5: E123 closing bracket does not match indentation of opening bracket's line
architv commented 8 years ago

@Saturn I've fixed the remaining pep8 errors except E731 (do not assign a lambda expression, use a def)

Saturn commented 8 years ago

I think the code looks fine now and formatting any more lines to make them less than 80 characters would be overdoing the pep8 instructions.

I agree. They are not unbreakable commandments :+1: