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

GitHub Action to lint Python code #1025

Merged
merged 2 commits into from
Apr 3, 2023
Merged

Conversation

cclauss
Copy link
Contributor

@cclauss cclauss commented Mar 21, 2023

Ruff supports over 500 lint rules and can be used to replace Flake8 (plus dozens of plugins), isort, pydocstyle, yesqa, eradicate, pyupgrade, and autoflake, all while executing (in Rust) tens or hundreds of times faster than any individual tool.

The ruff Action uses minimal steps to run in ~5 seconds, rapidly providing intuitive GitHub Annotations to contributors.

image

[Ruff](https://beta.ruff.rs/) supports [over 500 lint rules](https://beta.ruff.rs/docs/rules) including bandit, isort, pylint, pyupgrade, and flake8 plus its plugins, and is written in Rust for speed.

The `ruff` Action uses minimal steps to __run in ~5 seconds__, rapidly providing intuitive GitHub Annotations to contributors.

![image](https://user-images.githubusercontent.com/3709715/223758136-afc386d2-70aa-4eff-953a-2c2d82ceea23.png)
@RhetTbull
Copy link
Owner

Thanks. I am not currently using a linter on osxphotos so will need to see how much noise ruff makes before I merge this.

@cclauss
Copy link
Contributor Author

cclauss commented Mar 26, 2023

Please approve the 1 workflow awaiting approval below so we can make sure that it does not make any noise.

@RhetTbull
Copy link
Owner

Which rules are being ignored?

Also, why did you use 7228 for line length? (I admit, I hate linters that enforce short line lengths. I'm old enough to have used an 80 character phosphorous CRT and I don't understand why modern linters want to continue to enforce that when we're using 4K+ monitors and modern text editors).

@cclauss
Copy link
Contributor Author

cclauss commented Apr 1, 2023

I did not pick the number from thin air... I am sure that if we set it to 7227 and rerun the tests some super long line will raise an error. I am old enough to have used (in anger) 80-column punchcards so I merely set the line length at the limit of the codebase. That way if someone comes along and tries to create a new line that is 7229 chars long, they have to justify their excess in a code review.

@cclauss
Copy link
Contributor Author

cclauss commented Apr 1, 2023

tests/test_catalina_10_15_7.py:1360:201: E501 Line too long (7228 > 200 characters)

@RhetTbull
Copy link
Owner

That line (7227 chars) and a couple other long ones are test data. Eventually I should add a # noqa to those and use a more reasonable (but still >> 79 chars) line length.

@RhetTbull
Copy link
Owner

@all-contributors please add @cclauss for code

@allcontributors
Copy link
Contributor

@RhetTbull

I've put up a pull request to add @cclauss! 🎉

@RhetTbull RhetTbull merged commit 8b59615 into RhetTbull:main Apr 3, 2023
@cclauss cclauss deleted the patch-1 branch April 3, 2023 05:21
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.

None yet

2 participants