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

Fix a few performance and code quality issues #173

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

Fix a few performance and code quality issues #173

wants to merge 7 commits into from

Conversation

de-sh
Copy link

@de-sh de-sh commented Nov 14, 2020

Hi, I had recently run through your project and decided to check it with a static analyzer and found some interesting and concerning issues related to performance, bugs and security flaws. You can find the report here. I have used DeepSource's auto-fixer to fix a few, I hope to work on the rest soon.

Changes:

  • Remove unnecessary list comprehension
  • Remove unnecessary call to dict()
  • Remove unnecessary if statement in assignment
  • Add .deepsource.toml file to file to run continuous static analysis on the repository with DeepSource

I strongly recommend that you do enable DeepSource analysis after merging this PR, as it is free for OpenSource projects. If you are interested, please follow these steps:

  • Sign up on DeepSource with your GitHub account and grant access to this repo.
  • Activate analysis on this repo.
  • You can also look at the docs for more details. Do let me know if I can be of any help!

@de-sh
Copy link
Author

de-sh commented Nov 14, 2020

BTW, going back in time, I see that DeepSource was earlier enabled on this repo. May I know if there were any issues you faced because of which you decided to remove the same.

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.

None yet

2 participants