refactor(ci): rely on pre-commit file arguments#3201
Conversation
hubcio
left a comment
There was a problem hiding this comment.
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.
7d60611 to
1d8fae2
Compare
Signed-off-by: StandingMan <jmtangcs@gmail.com>
1d8fae2 to
d55965e
Compare
Signed-off-by: StandingMan <jmtangcs@gmail.com>
d55965e to
3f2faec
Compare
|
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 |
|
/ready |
hubcio
left a comment
There was a problem hiding this comment.
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.
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>
abaa365 to
63ae93e
Compare
|
/ready |
Which issue does this PR close?
Closes #3195.
Rationale
Some pre-commit hook scripts still accepted
--stagedand usedgit diff --cachedto 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?
--stagedhandling from file-scoped hook scripts.git diff --cachedbased staged-file discovery.--cibehavior for CI / PR-diff based checks.--helpoutput to document bare local runs with:Note: Running these scripts with --ci outside a CI environment, where neither
GITHUB_BASE_REFnor CI is set, now falls back to checking all trackedfiles 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
Ran
AI Usage
Drafted changes and updated the pr-commit scripts based on human direction.
🚧: Not tested locally yet.
Yes. I reviewed the changes