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

chore: Refactor code quality issues #1320

Merged
merged 2 commits into from
Apr 28, 2021
Merged

chore: Refactor code quality issues #1320

merged 2 commits into from
Apr 28, 2021

Conversation

akshgpt7
Copy link
Contributor

@akshgpt7 akshgpt7 commented Mar 19, 2021

Description

Hey, I'm a Developer Outreach at DeepSource and ran DeepSource analysis on my fork of the repo. It found some interesting code quality and performance improvements to consider.

This PR fixes a few of the issues detected for you to assess if it is right for you.
Happy to provide the tweaks separately otherwise :)

Summary of changes

  • Simplify if statement.
  • Use sys.exit() instead of bare exit().
  • Remove duplicate imports.
  • Use literals to create data structures to avoid function calls.
  • Add .deepsource.toml file for continuous analysis on bug risks/performance/code-quality issues on new changes.

@lgtm-com
Copy link

lgtm-com bot commented Mar 19, 2021

This pull request introduces 1 alert when merging 73aa78a into 4979c3a - view on LGTM.com

new alerts:

  • 1 for Unused local variable

@auvipy
Copy link
Member

auvipy commented Mar 20, 2021

unit tests are failing

@akshgpt7
Copy link
Contributor Author

@auvipy I've fixed the errors. Thanks!

@auvipy
Copy link
Member

auvipy commented Mar 21, 2021

@thedrow I think we can add this, what do you think?

.deepsource.toml Outdated Show resolved Hide resolved
kombu/asynchronous/http/curl.py Outdated Show resolved Hide resolved
@akshgpt7
Copy link
Contributor Author

akshgpt7 commented Apr 5, 2021

@matusvalo any updates on this?

@thedrow
Copy link
Member

thedrow commented Apr 6, 2021

@akshgpt7 Please address the code review comments.

@lgtm-com
Copy link

lgtm-com bot commented Apr 7, 2021

This pull request introduces 1 alert when merging c5d285a into 7c34684 - view on LGTM.com

new alerts:

  • 1 for Unused local variable

@thedrow thedrow self-requested a review April 7, 2021 09:56
@thedrow thedrow added this to the 5.1.0 milestone Apr 7, 2021
@thedrow thedrow requested a review from matusvalo April 7, 2021 09:56
@thedrow thedrow merged commit 3b6cd13 into celery:master Apr 28, 2021
matusvalo added a commit to matusvalo/kombu that referenced this pull request May 19, 2021
matusvalo added a commit that referenced this pull request May 19, 2021
matusvalo added a commit that referenced this pull request May 19, 2021
auvipy pushed a commit that referenced this pull request May 23, 2021
… KeyError (#1343)

* Fix broken nested() after #1320

* prepare_accept_content now raises SerializerNotInstalled when wrong serializer alias is passed
auvipy pushed a commit that referenced this pull request May 25, 2021
* Fix broken nested() after #1320

* Remove backward compatible code not needed anymore
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants