Skip to content

chore: add Ruff pre-commit checks#729

Draft
gabrielfruet wants to merge 2 commits into
mainfrom
gabriel-trn-1986-add-formatting-checks-to-pre-commit
Draft

chore: add Ruff pre-commit checks#729
gabrielfruet wants to merge 2 commits into
mainfrom
gabriel-trn-1986-add-formatting-checks-to-pre-commit

Conversation

@gabrielfruet

Copy link
Copy Markdown
Contributor

What has changed and why?

Added the existing Ruff checks to pre-commit so contributors can run the same Python
checks locally with one command.

Updated make format-check to use pre-commit and refreshed CONTRIBUTING.md to point
to the new workflow.

How has it been tested?

  • pre-commit run --all-files
  • make format-check
  • Manual hook proof with a temporary bad Python file:
    • ruff-format-check failed on bad formatting, passed after fixing
    • ruff-check failed on lint errors, passed after fixing

Did you update CHANGELOG.md?

  • Yes
  • Not needed (internal change)

Did you update the documentation?

  • Yes
  • Not needed (internal change without effects for user)

@gabrielfruet gabrielfruet marked this pull request as ready for review May 13, 2026 10:46
Copilot AI review requested due to automatic review settings May 13, 2026 10:46

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ad0276fdac

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .pre-commit-config.yaml Outdated
name: ruff format (check)
entry: ruff format --check
language: system
types_or: [python]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Include notebooks in Ruff hook file filters

The new local Ruff hooks are restricted to types_or: [python], which means pre-commit will not pass .ipynb files to either ruff format --check or ruff check. This is a regression from the previous ruff ... . commands in format-check, which did include notebooks (Ruff currently discovers examples/notebooks/*.ipynb by default). As a result, notebook code can now bypass lint/format checks in both pre-commit run --all-files and make format-check.

Useful? React with 👍 / 👎.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR integrates the repository’s existing Ruff formatting/linting checks into pre-commit, updates make format-check to delegate to pre-commit, and refreshes contributor documentation to reflect the new workflow.

Changes:

  • Updated make format-check to run pre-commit run --all-files instead of invoking Ruff directly.
  • Added local pre-commit hooks for ruff format --check and ruff check.
  • Updated CONTRIBUTING.md to recommend running pre-commit-based checks before committing.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
Makefile Switches format-check to run pre-commit hooks (instead of direct Ruff commands).
CONTRIBUTING.md Updates the recommended local workflow to include pre-commit.
.pre-commit-config.yaml Adds Ruff check hooks to pre-commit (local/system hooks).
Comments suppressed due to low confidence (2)

.pre-commit-config.yaml:30

  • language: system for this Ruff hook makes the pre-commit run depend on whatever ruff is available on the user's PATH at commit time (e.g., it can fail when the virtualenv isn’t activated, and it doesn’t pin a Ruff version). Consider using the official astral-sh/ruff-pre-commit repo (pinned rev) or a non-system hook setup so pre-commit manages the Ruff environment consistently.
      - id: ruff-check
        name: ruff check
        entry: ruff check
        language: system
        types_or: [python]

.pre-commit-config.yaml:31

  • With types_or: [python], this hook will not run on Jupyter notebooks (.ipynb). Previously ruff format --check . / ruff check . (in make format-check) would also cover notebooks under examples/notebooks, so this is a behavior change and may miss issues there. Consider expanding the hook to include .ipynb files (e.g., via an appropriate types/files matcher).
        entry: ruff check
        language: system
        types_or: [python]
        description: "Run Ruff lint checks."

Comment thread .pre-commit-config.yaml Outdated
Comment on lines +18 to +29
- repo: local
hooks:
- id: ruff-format-check
name: ruff format (check)
entry: ruff format --check
language: system
types_or: [python]
description: "Ensure Python files are formatted with Ruff."
- id: ruff-check
name: ruff check
entry: ruff check
language: system
Comment thread .pre-commit-config.yaml Outdated
Comment on lines +22 to +25
entry: ruff format --check
language: system
types_or: [python]
description: "Ensure Python files are formatted with Ruff."
Comment thread CONTRIBUTING.md Outdated
Comment on lines 22 to 29
Before committing code, make sure the checks pass. The common file-level checks are
handled by pre-commit, which `make install-dev` installs for you:

```
pre-commit run --all-files
make format
make static-checks
```
@gabrielfruet gabrielfruet marked this pull request as draft May 18, 2026 07:23
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