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

flake8, ref #8 #62

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

flake8, ref #8 #62

wants to merge 1 commit into from

Conversation

nim65s
Copy link
Contributor

@nim65s nim65s commented Nov 28, 2018

Hi again,

This PR follows #61, after I found out that some python scripts might not work on python 3.

Then, to ensure that there is no syntax errors on those script neither in python2 nor 3, I used the flake8 linter with python2 -m flake8 and python3 -m flake8.

Most changes are:

  • whitespaces, around operators, after a #, and some tabs have been replaced by spaces. To see this PR more clearly, you can ignore this by adding ?w=1 at the end of the url
  • using the print function
  • removing unused imports
  • splitting lines that are too long

@nim65s
Copy link
Contributor Author

nim65s commented Nov 28, 2018

NB: flake8 still complains a bit about function name should be lowercase, but I did not want to change any functionality in the code

@stephanemagnenat
Copy link
Collaborator

stephanemagnenat commented Nov 28, 2018

Thank you for your PR!

Using tabs instead of whitespaces is the Enki/Aseba style. Similarly, keeping relatively long lines is as well, as most editors have good soft warp. At that point, I do not want to change this style as it has been working well for a long time, and re-indenting everything would both loose the blame history and break ongoing refactoring in the ecs-refactor branch.

Can you please submit a PR with only the necessary changes (e.g. changes in print) and no style change? Thank you!

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