Skip to content

PEP8 cosmetic changes #3

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

Open
wants to merge 36 commits into
base: master
Choose a base branch
from
Open

Conversation

UkcaGreen
Copy link

Made cosmetic changes according to PEP8 by using "autopep8" tool.

Made cosmetic changes according to PEP8 by using "autopep8" tool.
Copy link
Member

@ceyonur ceyonur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok in short we need to refactor splitted lines. I put comments on some lines, and there are other similar lines needs to address this splitting. We don't need to strictly follow PEP8 if they're long lines. If they're really long lines try to split them by following other methods such as moving long parameters to new variables, splitting lines after/before new function calls. Instead of using different lines for each parameter maybe they can be grouped in multiple (2 parameters in each line etc...).

Just realized that you used autopep8 tool, if these changes are just made by the tool (not by you) then maybe you can consider to use the tool without telling it to "split the lines"?

Copy link
Member

@ceyonur ceyonur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added comments, there were some bugs and typos. Overall it obeys to PEP8, however some places are aggresively broken to multi-lines. Pep tool tries to wrap the lines after 79 characters but in some places it did some ugly breaks like breaking immediatly after paranthesis. I commented some places, but overall I've seen such cases in almost every file. If editing whole code seems overwhelming, we can split each file one by one and iterate over them. So you can work on one file then push it and get reviews & merge, then you can proceed other files.

Line breaks after open paranthesis are fixed.
Adjustifications about line length.
Adjustifications abot line length.
Adjustifications abot line length.
Newlines after open pharanthesis are fixed.
Line length adjustments.
Newline after open paranthesis are fixed.
Line adjustifications.
Newline after open paranthesis are fixed.
Help message passed as a variable.
@UkcaGreen
Copy link
Author

I have fixed the mentioned problems and similar cases in the pull request.

@ceyonur
Copy link
Member

ceyonur commented Mar 3, 2020

@UkcaGreen I've made some comments on the last review, yet I could not see any related changes for my comments. Have you forgotten to push some commits?

UkcaGreen and others added 11 commits March 5, 2020 00:40
newline after open pharanthesis is fixed.
newline after open pharanthesis is fixed
getting rid of variable with ambigious name.
Final touches in some files
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.

2 participants