Skip to content

chore: add husky pre-commit hook with lint-staged#759

Draft
hydrosquall wants to merge 1 commit intovega:mainfrom
hydrosquall:cameron.yick/add-pre-commit-lint-hooks
Draft

chore: add husky pre-commit hook with lint-staged#759
hydrosquall wants to merge 1 commit intovega:mainfrom
hydrosquall:cameron.yick/add-pre-commit-lint-hooks

Conversation

@hydrosquall
Copy link
Copy Markdown
Member

@hydrosquall hydrosquall commented Feb 8, 2026

Motivation

This PR adds pre-commit hooks to catch formatting issues locally before CI, reduce wasted CI minutes, and ensure consistent code style across the codebase (related to vega/vega#4237).

Changes

  • Adds husky@^9.1.7 for git hooks management
  • Adds lint-staged@^16.2.7 to run linters only on staged files
  • Configures pre-commit hook to run npx lint-staged
  • Adds "prepare": "husky" script to auto-initialize hooks on npm install
  • Configures lint-staged to run Python linting and formatting with ruff:
    • uv run ruff check --fix - auto-fixes linting issues
    • uv run ruff format - formats code to project standards
  • Uses uv run to ensure correct tool versions from uv.lock (per project standards)
  • Updates .gitignore to exclude .claude/ directory

Note: Taplo is excluded from pre-commit hooks due to maintainer stepping away and ARM Linux compatibility concerns, though it remains in CI.

Why Python/ruff instead of eslint format?

As noted in review feedback, this repository has a small TypeScript surface (4 files, 2 auto-generated) but significant Python development (11+ scripts). Recent CI failures have been in the "Format and lint (python)" step, not TypeScript.

Testing

Expand for local testing instructions
  1. Verify husky initialization:

    npm install
    # Should see husky install hooks
  2. Test the pre-commit hook with formatting issues:

    # Create a test file with bad formatting
    cat > test_formatting.py << 'PYEOF'
    def badly_formatted(x,y,z):
        result=x+y+z
        return result
    PYEOF
    
    # Try to commit it
    git add test_formatting.py
    git commit -m "test: formatting"
    
    # Should see ruff auto-fix the formatting issues
    # Check the file was reformatted:
    cat test_formatting.py
    
    # Clean up
    git rm test_formatting.py

Enable formatting issues to be automatically handled locally
instead of only erroring in CI, reducing wasted CI minutes.

Related to vega/vega#4237

Co-Authored-By: Claude Sonnet 4.5 (1M context) <[email protected]>
@dsmedia
Copy link
Copy Markdown
Member

dsmedia commented Feb 8, 2026

Hi @hydrosquall — am very supportive of pre-commit hooks. I wonder though if vega-datasets might need a different approach?

The TypeScript surface in this repo is very small (4 files in src/, two of which are auto-generated). The primary development here consists of Python scripts and TOML config. In fact, looking at recent CI failures, the bottleneck is the "Format and lint (python)" step, not TypeScript:

Run 21546775137 (ruff failure)

Run 21695659624 (ruff failure)

Also, just wanted to check about running tsc on specific staged files (which lint-staged does). This forces tsc to ignore tsconfig.json, losing the full project context.

I'd suggest somehow targeting ruff instead. Maybe like this.

"lint-staged": {
  "*.py": ["ruff check --fix", "ruff format"]
}

A note on Taplo: We might want to leave it out of the hook for now. The original maintainer stepped away from the project in December 2024. While v0.10.0 includes a Linux aarch64 binary, it was never published to PyPI. The latest PyPI version (0.9.3) lacks a Linux ARM wheel, which would break the pre-commit hook for developers on that platform. I'm hopeful v0.10.0 can get published to PyPI soon and if so that would hold us for a while.

All this said, I'm very much in favor of finding a suitable way to add pre-commit hooks for this repo.

@hydrosquall
Copy link
Copy Markdown
Member Author

hydrosquall commented Feb 8, 2026

@dsmedia thanks for the feedback and looking up the recent issues in CI that were specific to this repo.

Even though we could opt to run both, I agree ruff is probably the more useful formatter here and it might be enough to just run one given there are only 2 non-auto-generated TS files in this repo! I've adjusted the PR based on your comments.

@hydrosquall hydrosquall self-assigned this Feb 8, 2026
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