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

Fix some (critical) issues discovered by deepsource #919

Open
Byron opened this issue Sep 11, 2019 · 12 comments
Open

Fix some (critical) issues discovered by deepsource #919

Byron opened this issue Sep 11, 2019 · 12 comments

Comments

@Byron
Copy link
Member

Byron commented Sep 11, 2019

https://deepsource.io/gh/gitpython-developers/GitPython

@imkaka
Copy link
Contributor

imkaka commented Oct 15, 2019

Hey @Byron, I will take this. :)

Q1 - Why runtime_version = '2.x.x' in deepsource.toml?.

There are many issues which are okey without a fix, How one should make a call about that?

@Byron
Copy link
Member Author

Byron commented Oct 15, 2019

@imkaka Thanks a lot for your help.

When it's about making decisions, it is entirely up to you to make the final call. It should be possible to silence deepsource using an annotation, or to disable entire classes of errors/warnings.

Regarding Q1, also I don't know why the version is made explicit there. Maybe this is how they avoid drastic changes in behaviour? A quick check revealed that there is version 3.0 already, maybe consider upgrading before you begin.

Generally, you can feel free to make fixes the way you find appropriate :).

Good luck!

@imkaka
Copy link
Contributor

imkaka commented Oct 15, 2019

Do we only support Python 3 Now? then we should change the runtime_version='3.x.x'?

@Byron
Copy link
Member Author

Byron commented Oct 15, 2019

That's correct. Python 2 support was dropped not too long ago.

@imkaka
Copy link
Contributor

imkaka commented Oct 17, 2019

We have more issues after changing the version, I will try to fix this incrementally :)

@imkaka imkaka mentioned this issue Oct 23, 2019
2 tasks
@imkaka
Copy link
Contributor

imkaka commented Oct 23, 2019

In every file there are various Unused Imports as well as Wild Card Imports, Can anyone suggest do we use transitional imports?

Example:

  • a.py
from gitdb import X, Y
  • b.py
from a import X

What to do with Unused Imports?

@Byron
Copy link
Member Author

Byron commented Oct 24, 2019

Thanks for the concise explanation, @imaka. I understand that currently deepsource is not able to understand transitive imports within the same project. And even if it did, it would be impossible to know who else relies on unused imports from client projects.

Thus I would consider the status quo as part of the public interface GitPython provides to not risk any breakage.

Is it possible to disable this lint globally?

@dolftax
Copy link

dolftax commented Oct 25, 2019

This might help: https://deepsource.io/docs/configuration/skip-cq.html

Or you may do it via the UI as well - https://deepsource.io/blog/releases-issue-actions/

@imkaka
Copy link
Contributor

imkaka commented Oct 25, 2019

working on it. 💯

@imkaka
Copy link
Contributor

imkaka commented Oct 25, 2019

To silent globally, we have to log in to deepsource and select actions for each issue we want to disable as described in this.

I don't see the action button as I don't have credentials maybe.
@Byron

@Byron
Copy link
Member Author

Byron commented Oct 28, 2019

Thanks for the hints! It would be great to be able to ignore things globally using a file instead, keeping all relevant configuration with the project code.

For now, I have disabled all import related rules, and this is how it looks for me:
Screenshot 2019-10-28 at 08 48 54

@dolftax
Copy link

dolftax commented Oct 28, 2019

Thanks for the input @Byron. Will keep you posted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants