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 with codespell and ruff #41

Closed
wants to merge 1 commit into from

Conversation

cclauss
Copy link
Contributor

@cclauss cclauss commented Jul 8, 2023

@cclauss cclauss force-pushed the ruff branch 9 times, most recently from 59b167c to 4ab4a83 Compare July 9, 2023 05:20
@dbohdan
Copy link
Member

dbohdan commented Jul 9, 2023

Merged in 589594c. Thanks for the PR; I've been meaning to start using Ruff, and your PR prompted me to. I am not sure I will maintain this pipeline, but I think I will keep using Ruff. IMO jobs like this should run on all branches (consider a PR against a feature branch, for example), so I have changed that.

Some feedback. I recommend defaulting to "50/72 formatting" in GitHub PRs and Git commits in general. When you work with repos that use a variant of the Angular commit message format (like this one) or the Conventional Commits spec, it saves maintainers time if you also use it.

@dbohdan dbohdan closed this Jul 9, 2023
@cclauss cclauss deleted the ruff branch July 9, 2023 08:27
@cclauss
Copy link
Contributor Author

cclauss commented Jul 9, 2023

I would highly recommend setting up both https://pre-commit.com and https://pre-commit.ci as they save a ton of time.

I contribute to way too many repos that have widely varying commit formats and rebase rules to find a universal standard.

@cclauss
Copy link
Contributor Author

cclauss commented Jul 9, 2023

I usually add a commit message like this to explain why I set up the GitHub Action this way...

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

@dbohdan
Copy link
Member

dbohdan commented Jul 9, 2023

I would highly recommend setting up both https://pre-commit.com/ and https://pre-commit.ci/ as they save a ton of time.

Thanks for the suggestion. I'll check them out.

I contribute to way too many repos that have widely varying commit formats and rebase rules to find a universal standard.

While I don't have statistics to back this up, I am convinced that more than half the projects with some degree of exposure (for example, repositories with over 100 stars) follow the 50/72 rule. Adopting the format seems like pure gain and no loss to me. Just about nobody will complain about an under-50-character the first line of the commit message being too short. It amounts to being conservative in what you send.

I can't insist you do your work a certain way, of course, but I thought I'd offer my best argument for the 50/72 format.

This pull request was closed.
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