Repository navigation
chore(vdev): only report files modified by cue fmt in check docs - #26725
patrickswedish wants to merge 1 commit into
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
I have read the CLA Document and I hereby sign the CLA |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 25e6918f71
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| .map(String::from) | ||
| let mut modified: Vec<String> = after | ||
| .into_iter() | ||
| .filter(|(path, content)| before.get(path) != Some(content)) |
There was a problem hiding this comment.
On Git-for-Windows checkouts with core.autocrlf=true, tracked CUE files begin as CRLF and CUE's formatter emits canonical LF bytes; the documented formatter explicitly warns that ./scripts/cue.sh fmt may "rewrite any files" (docs/DOCUMENTING.md:104-116). The old git diff --name-only compared Git-normalized contents and therefore returned no path for an EOL-only rewrite, but this raw Vec<u8> comparison reports all formatted files and makes exec bail on every fresh checkout even though there is no committable diff. Normalize line endings or compare Git-normalized contents.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed: Added CRLF-to-LF normalization (
ormalize_crlf) to avoid false-positive diffs on checkouts with core.autocrlf=true.
| } | ||
|
|
||
| fn snapshot_cue_files(repo_root: &Path, source_dir: &Path) -> Result<BTreeMap<String, Vec<u8>>> { | ||
| let files = cue_files(source_dir)?; |
There was a problem hiding this comment.
On a checkout whose absolute path contains a glob metacharacter, such as /work/vector[1], this new snapshot call fails before cue fmt: cue_files interpolates source_dir.display() directly into the pattern at vdev/src/commands/build/docs_json.rs:85-93, so [1] is parsed as a character class and the actual source tree is not selected, usually triggering No CUE sources found. The old Git-diff implementation did not glob the checkout path; escape the literal prefix or walk the tree instead.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed: Replaced globbing with recursive directory walking (collect_cue_files) so paths containing brackets or glob metacharacters are safely traversed without pattern interpolation.
pront
added
the
no-changelog
25e6918 to
d5ef304
Compare
|
I have read the CLA Document and I hereby sign the CLA Updated branch with Codex suggestions addressed:
|
d5ef304 to
4ec2826
Compare
This branch has not been deployed
2 participants
Summary
Fixes #26707
Currently,
cargo vdev check docsruns./scripts/cue.sh fmtand then checksgit diff --name-only website/cue. When a developer has pre-existing unstaged (or untracked) edits underwebsite/cuethat are already properly formatted,git diff --name-onlystill lists those files and causescargo vdev check docsto fail with a false formatting error before reachingcue vet.This change snapshots the contents of the
.cuefiles underwebsite/cuebefore and after running./scripts/cue.sh fmt, and only reports files whose contents were actually modified by the formatter.Change Type
Checklist