Ruff formatter and Pre commit

[FallenDeity]

Change description

Replaced black with ruff for formatting and also added pre-commit for enforcing consistency (locally and via circle ci) across files for formatting and linting

Most changes are just formatting changes in terms of trailing whitespace/newline, which is enforced by pre-commit now and few other hooks

- repo: https://github.com/pre-commit/pre-commit-hooks
    rev: v4.3.0
    hooks:
      - id: check-ast
      - id: check-json
      - id: check-toml
      - id: check-yaml
        args:
          - --allow-multiple-documents
      - id: end-of-file-fixer
      - id: fix-byte-order-marker
      - id: pretty-format-json
        args:
          - --no-sort-keys
          - --no-ensure-ascii
      - id: sort-simple-yaml
      - id: trailing-whitespace
        args:
          - --markdown-linebreak-ext=md

cc: @phalt @Naramsim

AI coding assistance disclosure

None

Contributor check list

• I have written a description of the contribution and explained its motivation.
• I have written tests for my code changes (if applicable).
• I have read and understood the AI Assisted Contribution guidelines.
• I will own this change in production, and I am prepared to fix any bugs caused by my code change.

[FallenDeity]

also regarding coverage i noticed its not being used anywhere should i add coverage report generation with the tests or remove it from pyproject

[Naramsim]

Can we get rid of this target? isn't pre-commit installed along the other dependencies?

[FallenDeity]

that dosent install pre-commit the dependency but pre-commit the hook so whenever you next run git commit the pre commit checks defined in the config file automatically run

[Naramsim]

ah, got it! Ok, so, I don't want the hook on git. Or maybe to have it more complicated I'd like to have it only when uv is installed. I don't want to force everyone to install uv. If a contributor needs to submit a small PR not touching the python logic I'd like not to force the contributor to install all the necessary tools (python, uv, deps)

@jemarq04 @phalt what do you think?

[FallenDeity]

but arent both make install and make pre-commit-install optional like you decide whether to run it or not, although I get your point once the hooks are installed you need to run pre-commit uninstall to remove them if you want to in your next minor change or commit with --no-verify flag to skip the hooks

[phalt]

I think forcing pre-commit on everyone is a difficult sell.

But if we can have an optional path of using pre-commit with git, that is good with me :)

[jemarq04]

The changes we've already implemented do require uv to run things locally, which I think is a perfectly fine step forward. (I was using it myself as I didn't want to globally install some of the dependencies for this project!)

As far as pre-commit, the targets in the makefile seem like the right way to go. I usually like running it manually, so I'd skip the pre-commit-install target and just run it directly, which is possible with this setup. I like it!

[FallenDeity]

I think I can also specify explicitly in the readme/contributing md files that installing the hooks into git hooks is optional and add an emphasis that you can run it manually too so new contributors don't get confused if they aren't too familiar with pre-commit

[jemarq04]

Honestly, looking at the .pre-commit-config.yaml file, having it run automatically with every commit might be intensive! There's a check that builds and tests the entire database, which can be very lengthy. Having that run every time you commit may be too much. Maybe leave this off as a target and if someone wants it they can run pre-commit directly to install the hooks to git?

[FallenDeity]

https://github.com/PokeAPI/pokeapi/pull/1602/changes#diff-63a9c44a44acf85fea213a857769990937107cf072831e1a26808cfde9d096b9R60 the build and test is marked as manual so it doesn't run every commit rest of the stuff is just formatting checks where all of them together take like 3-5secs

to run a manual hook you need to explicitly invoke via a pre-commit hook id as such pre-commit run <hook_id> --all-files or via hook stage for all manual stages

[jemarq04]

Ah ok, I was unfamiliar with the manual key and didn't process that. With that in mind disregard my last comment - I think the pre-commit checks are great and having the install and run be separate targets is a good way to go. Thank you for clarifying!

[jemarq04]

As I mentioned in another comment, the pre-commit checks include a full "build-and-test" run. Adding it to the CI would be re-doing these tests?

Would it make sense to leave this off of the pre-commit altogether and otherwise leave the Makefile as-is?

[FallenDeity]

https://github.com/PokeAPI/pokeapi/pull/1602/changes#diff-63a9c44a44acf85fea213a857769990937107cf072831e1a26808cfde9d096b9R60

since its marked as manual its automatically skipped in the ci too along with ruff format since the ci already does that

this is the default suite that runs https://app.circleci.com/pipelines/github/PokeAPI/pokeapi/2512/workflows/5be65b3b-415b-46ad-87db-634725d10589/jobs/6139/parallel-runs/0/steps/0-104

[jemarq04]

Ok great - I mentioned this in the other comment. Thanks for the clarification!