chore: make pre-commit ruff hooks package-aware for monorepo - #18285
Open
chalmerlowe wants to merge 2 commits into
Open
chore: make pre-commit ruff hooks package-aware for monorepo#18285chalmerlowe wants to merge 2 commits into
chalmerlowe wants to merge 2 commits into
Conversation
Contributor
There was a problem hiding this comment.
Code Review
This pull request replaces the remote Ruff pre-commit hook with a custom, package-aware local runner script (ci/run_ruff.py) to handle linting and formatting within individual package directories in the monorepo. The review feedback highlights critical improvements for the pre-commit configuration, including adding additional_dependencies to pin the Ruff version in the isolated virtualenv and using python instead of python3 for Windows compatibility. Additionally, the feedback suggests handling potential FileNotFoundError exceptions when executing Ruff and expanding the file suffix checks to support .pyi and .ipynb files.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
chore: make pre-commit ruff hooks package-aware for monorepo
The Problem
The repository contains over 280 packages that share the
googlenamespace (such asgoogle.cloud.secretmanager,google.api_core, andgoogle.cloud.storage).When developers run pre-commit hooks locally, Ruff executes from the repository root. Because there is no single top-level
googlesource folder at the root, Ruff's import sorter (isortruleI001) cannot determine that package-local code is first-party. It treats all imports undergoogle.*as third-party dependencies and combines them into one alphabetical list.However, continuous integration lint sessions (
nox -s lint) execute from within each individual package folder (such aspackages/google-cloud-secret-manager). In that environment, Ruff recognizes the package's local code as first-party and requires third-party dependencies (likegoogle.protobuforgoogle.auth) to appear before local imports, separated by a blank line.This mismatch caused commits formatted by local pre-commit hooks to fail continuous integration lint tests.
The Solution
ci/run_ruff.py: A lightweight runner script that groups staged Python files by their enclosing package directory (underpackages/orpreview-packages/) and executesruff checkandruff formatwithin each package directory. For repository-level files outside of packages, it executes from the repository root..pre-commit-config.yaml: Replaced the generic root Ruff hooks with local hooks that callci/run_ruff.py.Notes to Reviewers
pre-commit runworks automatically out of the box.Irule selection used in the package Noxfiles.