Fix some (critical) issues discovered by deepsource

gitpython-developers / GitPython

GitPython is a python library used to interact with Git repositories.

http://gitpython.readthedocs.org

BSD 3-Clause "New" or "Revised" License

4.57k stars 901 forks source link

Fix some (critical) issues discovered by deepsource #919

Open Byron opened 5 years ago

Byron commented 5 years ago

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

imkaka commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

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

Byron commented 4 years ago

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

imkaka commented 4 years ago

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

imkaka commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

working on it. :100:

imkaka commented 4 years ago

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 commented 4 years ago

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:

dolftax commented 4 years ago

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