Skip to content
This repository has been archived by the owner on May 1, 2020. It is now read-only.

[MRG] Solve some pep8/flake8 issues & some code refactoring #25

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

[MRG] Solve some pep8/flake8 issues & some code refactoring #25

wants to merge 10 commits into from

Conversation

mohamed-ali
Copy link

@mohamed-ali mohamed-ali commented Apr 10, 2019

The purpose of this PR is to improve the code quality by reducing the number of alerts thrown by flake8:

$ flake8 . | wc -l 
     334

After the PR:

$ flake8 . | wc -l 
     235

The fourth commit reduces further the number of flake8 issues:

$ flake8 --exclude ./python/lopq/lopq_model_pb2.py . | wc -l 
     155

With commit 5-> 7 the number of issues is further reduced:

$ flake8 --exclude ./python/lopq/lopq_model_pb2.py . | wc -l 
     112

Down to 57 with the commit 8:

$ flake8 --exclude=./python/lopq/lopq_model_pb2.py .  | wc -l 
      57

The commits 9 and 10 finish fixing all identified flake8 issues:

$ flake8 --exclude=./python/lopq/lopq_model_pb2.py . | wc -l 
       0

PS: lopq_model_pb2.py is skipped because it's autogenerated by protobuf

@coveralls
Copy link

coveralls commented Apr 10, 2019

Coverage Status

Coverage remained the same at 88.443% when pulling 031d4b6 on mohamed-ali:fix-some-flake8-issues into 0f17655 on yahoo:master.

@pumpikano
Copy link
Collaborator

Thanks a lot for this! It looks excellent. Actually I am no longer an admin of this repo since I no longer work at Yahoo, so I currently cannot do the merge. However, I am working out an arrangement so that I can continue to maintain this. I'll follow up here soon with an update and we can get this and your other PR merged. Thanks.

@mohamed-ali mohamed-ali changed the title [MRG] Solve some pep8/flake8 issues [MRG] Solve some pep8/flake8 issues & some code refactoring Apr 13, 2019
@mohamed-ali
Copy link
Author

Hi @pumpikano, is there any progress on the status/maintainability of this library? Thanks.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants