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

Fixed prospector lint CI job #497

Merged
merged 1 commit into from
Sep 23, 2023
Merged

Conversation

benkonrath
Copy link
Member

No description provided.

@claudep
Copy link
Member

claudep commented Sep 23, 2023

Looks good, thanks! If recent prospector/pylint are too invasive, maybe we should simply go back to some simpler linter (flake8 or ruff). I'm not a fan of black, by the way.

@claudep claudep merged commit 92cffd9 into django:master Sep 23, 2023
6 checks passed
@benkonrath
Copy link
Member Author

I'm open to something simpler, however, I think other linters may have the same issue as prospector; upgrading always will add more rules and therefore take time. Perhaps prospector is a little more complex because it depends on so many of the Python linters.

I do actually like black but I see it more as a code formatter rather than a static analysis linter that tries find potential problems in the code. My argument for black is that it's nice not to waste time commenting on formatting in reviews or thinking about it when coding. When I first started using black, I didn't always like the style of code it produced but now I'm used to it and I don't really notice anymore.

The negative point for using black is that we'd loose git history if we ever did start using it. This is something I'd miss. In any case, I think all active maintainers of localflavor would need to be onboard for a change like this so we can just keep things the way they are (without black).

I'll revisit updating prospector or switching to flake8 when I have a little more spare time.

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

Successfully merging this pull request may close these issues.

2 participants