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

Using pre-commit framework #3213

Open
ccamara opened this issue Jan 12, 2024 · 0 comments · May be fixed by #3212
Open

Using pre-commit framework #3213

ccamara opened this issue Jan 12, 2024 · 0 comments · May be fixed by #3212

Comments

@ccamara
Copy link
Contributor

ccamara commented Jan 12, 2024

Is your feature request related to a problem? Please describe.
This is not really a problem, but a proposed new feature.
I've learnt the hard way that bookwyrm follow a style guide and has some tests that need to be passed before merging a PR.

Describe the solution you'd like
I'd like to have an automated way to get those tests immediately and in an automated way, without having to wait for the GithubActions output, to speed up the contributing process, to save time to PR reviewers and get rid of commits with just styling.

Describe alternatives you've considered
While theoretically this could be done implementing a git hook, I'm proposing implementing Pre-commit framework, which should run those tests automatically BEFORE creating a commit. While it may seem redundant, it is not:

  • CI tests help noticing any error
  • Pre-commit prevents errors from being pushed (this is, provided pre-commit is installed in the environment)
    • (optinal) Pre-commit could detect and fix errors already in the codebase which were introduced prior to the automated tests (run pre-commit run --all-files)
    • (optional) The same pre-commit config could be Integrates with CI/CD, via https://pre-commit.ci/ so the same code would be run locally and on CI

Additional context

  • I've created a draft PR here: Using pre-commit framework  #3212
  • I've never used pre-commit before, but this looks promising and I was looking for an opportunity to learn about it. IMHO this is the right context for this.
@ccamara ccamara linked a pull request Jan 13, 2024 that will close this issue
6 tasks
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 a pull request may close this issue.

1 participant