Skip to content

refactor(ci): rely on pre-commit file arguments#3201

Open
Standing-Man wants to merge 4 commits into
apache:masterfrom
Standing-Man:remove-staged-logic
Open

refactor(ci): rely on pre-commit file arguments#3201
Standing-Man wants to merge 4 commits into
apache:masterfrom
Standing-Man:remove-staged-logic

Conversation

@Standing-Man
Copy link
Copy Markdown
Contributor

@Standing-Man Standing-Man commented Apr 30, 2026

Which issue does this PR close?

Closes #3195.

Rationale

Some pre-commit hook scripts still accepted --staged and used git diff --cached to discover staged files internally.

That duplicated pre-commit's own file selection behavior. Since these hooks now use pass_filenames, pre-commit already passes the matched staged files directly to the scripts. Keeping a second staged-file discovery path made the behavior harder to reason about and easier to drift from pre-commit's filtering rules.

What changed?

  • Removed --staged argument handling from file-scoped hook scripts.
  • Removed git diff --cached based staged-file discovery.
  • Updated help text to no longer mention --staged.
  • Kept explicit file arguments as the primary input path for pre-commit.
  • Kept full-repository behavior for local/manual runs when no files are provided.
  • Preserved --ci behavior for GitHub Actions / PR-diff checks.

Local Execution

  • Passed / not passed
    Passed
  • Pre-commit hooks ran / not ran
    Ran

AI Usage

  1. Which tools? (e.g., GitHub Copilot, Claude, ChatGPT) Codex
  2. Scope of usage? (e.g., autocomplete, generated functions, entire implementation)
    Drafted changes and updated the pr-commit scripts based on human direction.
  3. How did you verify the generated code works correctly?
    🚧: Not tested locally yet.
  4. Can you explain every line of the code if asked?
    Yes. I reviewed the changes

@Standing-Man Standing-Man marked this pull request as ready for review April 30, 2026 13:55
Copy link
Copy Markdown
Contributor

@hubcio hubcio left a comment

Choose a reason for hiding this comment

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

behavior change worth noting in the commit body: running scripts with --ci outside CI (no GITHUB_BASE_REF, no CI env) now scans the whole repo instead of staged files. not a bug since pre-commit no longer relies on it, but a one-line note in the commit message would help anyone running the scripts manually.

Comment thread scripts/ci/taplo.sh Outdated
Comment thread scripts/ci/markdownlint.sh Outdated
;;
--all)
FILE_MODE="all"
shift
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.

--all is now a no-op (just shift, no FILE_MODE var anymore). help text on line 43 still says (default). flag is harmless but redundant - either drop it or document it as accepted-for-compat.

Comment thread scripts/ci/shellcheck.sh Outdated
;;
--all)
FILE_MODE="all"
shift
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.

same as markdownlint.sh - --all is now a no-op after FILE_MODE removal. help text still claims it's the default. either drop the flag or keep it as a documented compat shim.

@Standing-Man Standing-Man force-pushed the remove-staged-logic branch from 7d60611 to 1d8fae2 Compare May 6, 2026 13:14
Signed-off-by: StandingMan <jmtangcs@gmail.com>
@Standing-Man Standing-Man force-pushed the remove-staged-logic branch from 1d8fae2 to d55965e Compare May 6, 2026 13:15
Signed-off-by: StandingMan <jmtangcs@gmail.com>
@Standing-Man Standing-Man force-pushed the remove-staged-logic branch from d55965e to 3f2faec Compare May 6, 2026 13:17
@hubcio
Copy link
Copy Markdown
Contributor

hubcio commented May 14, 2026

if PR is ready to be reviewed again, please type comment with /ready somewhere in the beginning of any line. also comment/resolve the issues.

/author

@github-actions github-actions Bot added the S-waiting-on-author PR is waiting on author response label May 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-author PR is waiting on author response

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Align pre-commit hooks to use passed filenames and remove internal git diff logic

2 participants