Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix quality issues reported by DeepSource #574

Merged
merged 2 commits into from
May 23, 2019
Merged

Fix quality issues reported by DeepSource #574

merged 2 commits into from
May 23, 2019

Conversation

sauravsrijan
Copy link
Contributor

Fixes:

  • PYL-W0612: Rename unused variable to _
  • PYL-C0123: Use isinstance() for checking type
  • PAP-W0011: Use dict.items() to iterate over a dictionary
  • PAP-W0012: Use dict.get() to access value from dictionary instead of an if query to check whether the key exists
  • PYL-C0325: Remove superfluous brackets ()
  • PYL-R1701: Merge multiple consecutive isinstance() calls into one

Fixes:
- PYL-W0612: Rename unused variable to `_`
- PYL-C0123: Use `isinstance()` for checking type
- PAP-W0011: Use `dict.items()` to iterate over a dictionary
- PAP-W0012: Use `dict.get()` to access value from dictionary instead of an `if` query to check whether the key exists
- PYL-C0325: Remove superfluous brackets `()`
- PYL-R1701: Merge multiple consecutive `isinstance()` calls into one
@sauravsrijan
Copy link
Contributor Author

sauravsrijan commented May 23, 2019

Hi @ob-stripe, here are some more code quality issues fixed.
I would also suggest you to update the .deepsource.toml config file to make the analysis better and ignore issues which might not be important for you.

Suggestions:

  • In the config, add tests/** in test-patterns to ignore running analysis on test files.
  • Set max-line-length to 100 in the toml. You would need to add an [analyzers.meta] field and set it inside it. You can see an example here.

Copy link
Contributor

@ob-stripe ob-stripe left a comment

Choose a reason for hiding this comment

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

One minor request, but looks great! Thanks!

@sauravsrijan
Copy link
Contributor Author

Updated the PR. Good to go.

@ob-stripe
Copy link
Contributor

Awesome, thanks!

@ob-stripe ob-stripe self-assigned this May 23, 2019
@ob-stripe ob-stripe merged commit b355357 into stripe:master May 23, 2019
@ob-stripe
Copy link
Contributor

Released as 2.28.2.

@ob-stripe
Copy link
Contributor

In the config, add tests/** in test-patterns to ignore running analysis on test files.

@sauravsrijan I've already added tests/* per the example in https://deepsource.io/docs/configuration/analysis.html#test-patterns. Do I need ** so that it applies to all subdirectories as well?

@sauravsrijan
Copy link
Contributor Author

You have to do tests/** to discover files recursively.
tests/* will only match one level.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants