Closed HarshCasper closed 3 years ago
Hello @HarshCasper
Thanks for your PR
I have a few questions
Hi @srevinsaju
Thanks for taking the time to review this. The Commits were made by the DeepSource App and I pushed all the changes to a Single Commit.
If you would like, I can split the changes into smaller commits and send individual PRs for them to make it easy for them to review. If there is not a hard-fast requirement to squash the Commits before the merge, I will have the commits attributed to the Bot.
Since the DeepSource Analysis has pointers that feature issues that can be auto fixed or can be just manually fixed, I would love to work on the same.
@chimosky I guess it would be best, if I close this PR and start work on a fresh one adding discrete changes to the same. Kindly let me know if that would work.
@chimosky I guess it would be best, if I close this PR and start work on a fresh one adding discrete changes to the same. Kindly let me know if that would work.
That would be great, thanks.
Thanks. Reviewed.
@staticmethod
makes the source longer without adding substantive value, although the removal of else
has compensated somewhat,else
in places that use a premature return
is something I like, but we've had a history of sticking to classical structured coding, in the hope that our users may discover how Sugar works (hence the View Source feature and the Pippy activity),I've cherry-picked 7bbff9f5981("Deepsource: unnecessary else
or elif
") and 0388f516b ("Deepsource: Remove unused private method") and pushed to master.
This pull request fixes some of the quality issues raised by DeepSource on my fork of this repository.
Summary of fixes:
else
/elif
whenif
block has areturn
statementobject
from the base classYou can see all the issues raised by DeepSource here.
These Issues were fixed automatically by DeepSource
Using DeepSource to continuously analyze your repository
.deepsource.toml
in this PR, which you can use to configure your analysis settings.If you do not want to use DeepSource to continuously analyze this repo, I'll remove the
.deepsource.toml
from this PR and you can merge the rest of the fixes.