Skip to content

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

Merged
krishvishal merged 5 commits into
apache:masterfrom
Standing-Man:remove-staged-logic
May 20, 2026
Merged

refactor(ci): rely on pre-commit file arguments#3201
krishvishal merged 5 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 handling from file-scoped hook scripts.
  • Removed git diff --cached based staged-file discovery.
  • Kept explicit file arguments as the primary input path for pre-commit.
  • Preserved --ci behavior for CI / PR-diff based checks.
  • Updated --help output to document bare local runs with:

Note: Running these scripts with --ci outside a CI environment, where neither
GITHUB_BASE_REF nor CI is set, now falls back to checking all tracked
files instead of staged files.

Running the scripts with no file arguments also defaults to checking all
tracked files. This is intentional because pre-commit no longer relies on
script-side staged file discovery; it passes matched files directly via
pass_filenames.

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
Comment thread scripts/ci/shellcheck.sh Outdated
@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
@Standing-Man
Copy link
Copy Markdown
Contributor Author

/ready

@github-actions github-actions Bot added S-waiting-on-review PR is waiting on a reviewer and removed S-waiting-on-author PR is waiting on author response labels May 17, 2026
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.

nit outside the diff hunks: scripts/ci/shellcheck.sh:99-100 and 131-132 still say "Use staged or explicitly provided files when present". --staged was removed in this PR, so the "staged or" wording is misleading. drop it.

also, my prior comment about the --ci-outside-CI behavior change still applies - running scripts with --ci when neither GITHUB_BASE_REF nor CI is set now scans the whole repo via git ls-files instead of staged files. a one-line note in the commit body would help anyone running manually.

Comment thread scripts/ci/binary-artifacts.sh
@github-actions github-actions Bot added S-waiting-on-author PR is waiting on author response and removed S-waiting-on-review PR is waiting on a reviewer labels May 18, 2026
Document that bare local runs of file-scoped hook scripts check all
tracked files by default. This makes the manual-run behavior explicit
after removing script-side staged file discovery.

Signed-off-by: StandingMan <jmtangcs@gmail.com>
@Standing-Man Standing-Man force-pushed the remove-staged-logic branch from abaa365 to 63ae93e Compare May 20, 2026 14:02
@Standing-Man
Copy link
Copy Markdown
Contributor Author

/ready

@github-actions github-actions Bot added S-waiting-on-review PR is waiting on a reviewer and removed S-waiting-on-author PR is waiting on author response labels May 20, 2026
@krishvishal krishvishal merged commit 638e5fb into apache:master May 20, 2026
27 checks passed
@github-actions github-actions Bot removed the S-waiting-on-review PR is waiting on a reviewer label May 20, 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.

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

4 participants