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

Add Pylint #22

Open
JuanjoSalvador opened this issue Nov 4, 2017 · 2 comments
Open

Add Pylint #22

JuanjoSalvador opened this issue Nov 4, 2017 · 2 comments

Comments

@JuanjoSalvador
Copy link
Owner

Adding Pylint I can improve my code, that's a good practise.

https://pylint.org/

@Euklios
Copy link
Contributor

Euklios commented Apr 23, 2022

I just checked some of the pylint default warnings and got some questions:

How should we treat API changes?
For example: Nyaa.get and SukebeiNyaa.get have an inconsistently named parameter called id (or view_id in Nyaa.get). If we renamed that to view_id, which would be more specific, code like sukebei_nyaa.get(id="123") would break.

How should we handle the integration/automation of pylint?
The simplest thing would be running it manually; this is already possible.
We could also include it in a Pre-commit hook or run it from the CI. While the last option might be harder to set up, it could be interesting, depending on the number of automated tests planned.

We already talked about Pantsu; remove it from the project?
I don't want to investigate warnings in places that will be deleted anyway.

@JuanjoSalvador
Copy link
Owner Author

I'll take this, and I'm gonna think about how we can deal with this. At this moment I have some fixes on my local branches, but it needs some design changes.

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

No branches or pull requests

2 participants