Repository navigation
Conversation
In tailwindlabs#15941, the intention was to prevent .gitignore files outside of a git repository from taking effect while ensuring global ignore files configured in git (like core.excludesFile) continue to work. However, builder.git_global(false) was explicitly set in the oxide scanner walker, which caused git's global ignore configuration (core.excludesFile / ~/.config/git/ignore) to be completely ignored. This change enables git_global(true) so that global ignore rules configured in git are honored as expected during source detection. Fixes tailwindlabs#20509
[Medium impact] The PR appears safe to merge. Reviews (5) · Last reviewed commit: "Merge branch 'main' of github.com:tailwi..." · Reviewed by Greptile |
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughThe excludes-file parser now accepts quoted Priority: ➖ Normal Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Description check | ✅ Passed | The description directly explains the scanner change, its Git configuration behavior, the related issue, and the test plan. |
| Linked Issues check | ✅ Passed | The changes directly address issue #20509 by respecting Git global ignore rules during source scanning. |
| Out of Scope Changes check | ✅ Passed | The implementation and tests remain within the stated scope of enabling Git global ignore files and validating core.excludesFile behavior. |
| Title check | ✅ Passed | The title clearly identifies the main change: respecting Git global ignore files in the source scanner. |
| Linked Issues check | ✅ Passed | Issue [#20509] requires source scanning to skip files matched by Git core.excludesFile. create_walker now calls builder.git_global(true). The new respects_git_core_excludes_file test configure… |
| Out of Scope Changes check | ✅ Passed | The changes stay within issue [#20509]. The scanner setting implements global Git ignore support. The parser change supports valid core.excludesFile paths required by that support. The test provides… |
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/oxide/tests/scanner.rs (1)
2961-2961: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winSynchronize the process-wide
GIT_CONFIG_GLOBALmutation.
GIT_CONFIG_GLOBALis process-wide, and global Git-ignore handling reads it duringScanner::new. The configured pattern matches onlyignored-by-global.html. The inspected concurrent scanner fixtures do not contain that basename, and their Git commands are onlygit initcalls with ignored output. The race therefore does not currently change an asserted result or cause a command failure. It can affect a future scan that contains this basename. Isolate this test in a separate process or protect the mutation with a shared lock.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: 7f764ba3-4601-4e02-b611-e93f5ae31d2d
📒 Files selected for processing (2)
crates/oxide/src/scanner/mod.rscrates/oxide/tests/scanner.rs
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.
|
Hi maintainers, following up on this regression-test PR. It enables Git global ignore files during source scanning and adds coverage for core.excludesFile, addressing #20509. Greptile review is passing and the branch is mergeable; happy to adjust the implementation or test if there is any concern. |
The hooks are in git/hooks now, still found through core.hooksPath. That frees git/template to be an actual init.templateDir again. Sure, the hooks could've stayed behind, but then they'd be copied over on every `git init` and be the inert copies `git hooks-clean` removes. For now, this is only info/exclude with /.claude/ in it. But why, especially when ~/.gitignore (my core.excludesfile) already has it? Well, there's a bit of belt-and-suspenders action here, just super ensuring this doesn't get in unless I really, really want it to. Also, Tailwind v4 doesn't read the global excludes file, and it _does_ read .git/info/exclude. So something in some note can end up in the built CSS, even when it's not used in the app. Watch tailwindlabs/tailwindcss#20524 to see if/when that changes.
…espect-core-excludesfile
When scanning for source candidates, Oxide previously disabled git global ignore files via
builder.git_global(false).As noted in #15941, the intention was to prevent
.gitignorefiles in parent directories outside of a git repository from taking effect, while keeping global ignore files configured in git working. However, callinggit_global(false)on the walker had the unintended side effect of ignoringcore.excludesFile(and$XDG_CONFIG_HOME/git/ignore) entirely.This change enables
builder.git_global(true)so that ignore patterns configured in Git's global configuration are respected during source scanning.Fixes #20509
Test plan
respects_git_core_excludes_fileincrates/oxide/tests/scanner.rsverifying that files matched by Git'score.excludesFileare skipped.[ci-all]