-
Notifications
You must be signed in to change notification settings - Fork 215
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
Use pre-commit for linting #3177
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, but this file needs to be changed as well to make the pipeline work: https://github.com/LibreLingo/LibreLingo/blob/main/.github/workflows/python.yml
I added ruff as a poetry dependency and changed the workflow to use ruff instead of black/pylint |
hi @sublime-coleslaw it seems like it is still failing? https://github.com/LibreLingo/LibreLingo/actions/runs/6958213826/job/19029305201?pr=3177 are you having trouble running these checks in your fork? maybe I could help you with that |
whoops! forgot to check that command locally before pushing. unfortunately github doesn't seem to want to run any of the workflows in my fork on push, and i can't create a PR in my fork. if you know of a workaround, i'd appreciate help getting it to run :) |
If I understand correctly, it should be enough for you to enable Github Actions in your fork, and then it should run the github action using your fork. But that probably does not apply to the "pull request" trigger, only to the "push" trigger I guess. (I'm not sure about it) So I think this might actually be my fault, as it seems the |
#3159
This adds a dev dependency for pre-commit, and a basic pre-commit configuration for linting with ruff. The second commit fixes the various issues that pre-commit complained about.
Tests still aren't run using pre-commit, and none of the github actions have been switched to use pre-commit. I'm not sure if that would be a good idea since pre-commit usually isn't used to run tests.