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

chore: configure pre-commit hooks for code formatting #103

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v4.6.0
hooks:
- id: trailing-whitespace
- id: end-of-file-fixer
- id: check-yaml
- id: check-json
- id: check-added-large-files
- id: check-merge-conflict
Copy link
Member

Choose a reason for hiding this comment

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

are these hooks fully compatible with what we are already formatting via github actions workflow and make check-python/check-rust commands? if not, we need to make them behave the same. And if possible, reuse the same make commands for reusability

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we remove the checks for YAML, JSON, and file validations compared to GitHub Actions? Alternatively, should we add corresponding configurations in GitHub Actions? And I attempted to utilize make check-python, but encountered an issue where mypy did not recognize the configuration from pyproject.toml, which block me reuse make commands。


- repo: https://github.com/charliermarsh/ruff-pre-commit
rev: v0.2.0
hooks:
- id: ruff
args: [--fix]
- id: ruff-format

- repo: https://github.com/pre-commit/mirrors-mypy
rev: v1.11.0
hooks:
- id: mypy

- repo: local
hooks:
- id: cargo-fmt
name: cargo fmt
entry: cargo fmt --all -- --check
language: system
types: [rust]
pass_filenames: false
Copy link
Member

Choose a reason for hiding this comment

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

can you add a section in the CONTRIBUTING.md to describe and explain what kind of pre-commit changes will be make once a developer configures this pre-commit hook.

2 changes: 2 additions & 0 deletions python/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ setup-venv: ## Setup the virtualenv
setup: ## Setup the requirements
$(info --- Setup dependencies ---)
pip install "$(MATURIN_VERSION)"
pip install pre-commit
pre-commit install
Comment on lines +33 to +34
Copy link
Member

Choose a reason for hiding this comment

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

this should be a devel dependency in pyproject.toml

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it!


.PHONY: build
build: setup ## Build Python binding of delta-rs
Expand Down
Loading