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

Refactor code quality and performance issues #663

Closed
wants to merge 1 commit into from
Closed

Refactor code quality and performance issues #663

wants to merge 1 commit into from

Conversation

akshgpt7
Copy link

Description

Hi 👋, I ran DeepSource analysis on my fork of the repo, and found some interesting code quality issues that can improve the general code quality and performance here.
This PR fixes a few of the issues detected.

Summary of changes:

  • Remove repeated imports.
  • Remove duplicate key in dictionary.
  • Iterate directly over dictionary instead of calling keys().
  • Simplify if statements.
  • Refactor unnecessary use of list comprehension, affecting performance.

@JimmXinu JimmXinu closed this in 8a7423d Feb 24, 2021
@JimmXinu
Copy link
Owner

I've incorporated some of this, thanks. But I don't want to make any unnecessary changes to included_dependencies.

@akshgpt7
Copy link
Author

akshgpt7 commented Feb 24, 2021

That makes sense. Thanks!
Also, @JimmXinu, I'd just like to point out that the issues fixed here were detected by running DeepSource analysis on the repo. If you like, you can activate analysis for your repository to detect such code quality issues/bug risks on the fly for every change made. You can also use the Autofix feature to fix them with one click. As for the included_dependencies, there's an option to exclude file paths.

Here's what you can do if you wish to activate DeepSource to continuously analyze your repository:

  • Install DeepSource on your repository.
  • Create .deepsource.toml configuration which you can use to configure your analysis settings.
  • Activate analysis here.

If you have any doubts or questions, you can check out the docs, or feel free to reach out :)

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