refactor(ci): rely on pre-commit file arguments#3201
refactor(ci): rely on pre-commit file arguments#3201Standing-Man wants to merge 2 commits intoapache:masterfrom
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.
| # Fallback to staged files | ||
| git diff --cached --name-only --diff-filter=ACM -- '*.toml' | ||
| # Local fallback: check all TOML files | ||
| find . -name '*.toml' \ |
There was a problem hiding this comment.
the local fallback here uses find . while the sibling scripts (binary-artifacts.sh, trailing-newline.sh, trailing-whitespace.sh) use git ls-files for the same fallback. inconsistent. could align to git ls-files -- '*.toml' and also collapse the duplication with the all) branch below since they're now functionally identical.
There was a problem hiding this comment.
I’ve replaced find with git ls-files in the latest commit to collect the files to be checked.
| ;; | ||
| --all) | ||
| FILE_MODE="all" | ||
| shift |
There was a problem hiding this comment.
--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.
| ;; | ||
| --all) | ||
| FILE_MODE="all" | ||
| shift |
There was a problem hiding this comment.
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.
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
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?
--stagedargument handling from file-scoped hook scripts.git diff --cachedbased staged-file discovery.--staged.--cibehavior for GitHub Actions / PR-diff checks.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