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

improve code quality #3245

Merged
merged 7 commits into from
Apr 7, 2021
Merged

improve code quality #3245

merged 7 commits into from
Apr 7, 2021

Conversation

withshubh
Copy link
Contributor

Description

Hi 👋 I work at DeepSource, I ran DeepSource analysis on the forked copy of this repo and found some interesting code quality issues in the codebase, opening this PR so you can assess if our platform is right and helpful for you.

Summary of changes

  • Remove unnecessary comprehension
  • Remove unnecessary use of comprehension
  • Remove methods with unnecessary super delegation.
  • Iterate dictionary directly
  • Refactor the comparison involving not
  • Remove unnecessary generator
  • Add .deepsource.toml

Copy link
Member

@eukreign eukreign left a comment

Choose a reason for hiding this comment

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

Please rebase this and remove the .deepsource.toml config for now until we've had a chance to discuss it to better understand the implications of this service.

.deepsource.toml Outdated
enabled = true

[analyzers.meta]
runtime_version = "3.x.x"
Copy link
Member

Choose a reason for hiding this comment

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

Let's hold off on adding this config in this PR. We should discuss it to better understand the implications.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have removed the configuration file ✨

Shall I create an issue for the same?

@lbry-bot lbry-bot assigned eukreign and unassigned eukreign Mar 30, 2021
Signed-off-by: shubhendra <withshubh@gmail.com>
Signed-off-by: shubhendra <withshubh@gmail.com>
Signed-off-by: shubhendra <withshubh@gmail.com>
Signed-off-by: shubhendra <withshubh@gmail.com>
Signed-off-by: shubhendra <withshubh@gmail.com>
Signed-off-by: shubhendra <withshubh@gmail.com>
@lbry-bot lbry-bot assigned eukreign and unassigned eukreign Apr 7, 2021
@eukreign eukreign added area: other type: improvement Existing (or partially existing) functionality needs to be changed labels Apr 7, 2021
@eukreign eukreign changed the title fix code quality issues fix code quality Apr 7, 2021
@eukreign eukreign changed the title fix code quality improve code quality Apr 7, 2021
@eukreign eukreign merged commit ff30386 into lbryio:master Apr 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: other type: improvement Existing (or partially existing) functionality needs to be changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants