chore: normalize line endings (LF) and add .gitattributes rules#236
Conversation
In #235 (first no-panic burn-down), three shipper-core source files flipped from LF to CRLF in their committed form because: - The repo standard is LF (verified against pre-#235 storage). - The Windows working tree had CRLF (historical autocrlf side-effect from before this checkout). - `core.autocrlf` is `false` in this clone, so git stored whatever bytes were in the working tree. - The Edit tool preserved the working-tree CRLF when it rewrote the files, and the commit therefore stored CRLF. The diff in #235 showed ~4,000 line changes that were actually 15 lines of real code change plus full-file line-ending flips. Unreviewable. This PR locks the policy down: .gitattributes: `* text=auto eol=lf` — every text file is stored with LF regardless of platform. Working-tree files may still be CRLF on Windows; only storage matters. The existing `linguist-generated=true` rules for `policy/no-panic-baseline.json` and `badges/*.json` are preserved. crates/shipper-core/src/ops/cargo/mod.rs crates/shipper-core/src/plan/chunking/mod.rs crates/shipper-core/src/runtime/environment/mod.rs Renormalized back to LF via `dos2unix`. `git diff -w HEAD` against the prior commit produces zero non-whitespace differences — the file content is byte-identical to the pre-#235 state plus the three small semantic refactors #235 actually intended. After this lands, future Edit operations on these files will store LF on commit even when the working tree is CRLF; the .gitattributes rule makes git do the conversion at index time.
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
Droid finished @EffortlessSteven's task —— View job Validation Complete — PR #236 ✅Status: No actionable findings emitted. Inspected surfaces:
Why no comments: Residual risk: Validation signal:
Candidates reviewed: 0 |
…) (#237) Stabilises `concurrent_version_exists_checks` (and any other test using `with_multi_server`) on slow macOS CI runners. Hit three times in a single rollout session (#233, #234, #236) as `version_exists: registry request failed -> operation timed out` against the local tiny_http mock. Root cause: the helper's accept loop blocked on `handler(req)` until each response was fully written before returning to `recv_timeout`. With 5 concurrent reqwest clients hitting the same loopback socket, the remaining clients sat in the kernel's TCP backlog long enough to exceed reqwest's default OS-level connect timeout. Windows and Linux runners process the queue fast enough to mask the bug; macOS does not. Fix: spawn one worker thread per accepted request and let the accept loop return to `recv_timeout` immediately. The accept loop still serialises on `recv_timeout` (tiny_http requires that), but handlers run in parallel, so the kernel's listen queue drains as fast as connections arrive. Other changes: - `recv_timeout` bumped from 30s to 60s for additional headroom. - Trait bound on the handler closure goes from `Fn + Send + 'static` to `Fn + Send + Sync + 'static` (required to wrap the handler in `Arc` for clone-into-workers). All existing call sites use closures that already satisfy `Sync`. - The accept thread joins worker threads before returning so any panic in a handler surfaces in CI. cargo test -p shipper-registry --lib passes 258/258 locally (Windows). The fix targets a macOS-specific timing bug, so CI is the real verification.
Summary
Fixes the line-ending churn introduced in #235 and locks down LF as the repo policy via
.gitattributes.Why this is needed
In #235 (first no-panic burn-down), three
shipper-coresource files flipped from LF to CRLF in their committed form:git show 16977f7:crates/shipper-core/src/ops/cargo/mod.rs | head -c 100 | od -c).core.autocrlfisfalsein this clone, so git stored whatever bytes were in the working tree.The #235 diff showed ~4,000 "line changes" that were actually 15 lines of real code change plus full-file line-ending flips. Unreviewable.
What changes
.gitattributes* text=auto eol=lf— every text file is stored with LF regardless of platform. Existinglinguist-generated=truerules preserved.crates/shipper-core/src/ops/cargo/mod.rsdos2unix.crates/shipper-core/src/plan/chunking/mod.rsdos2unix.crates/shipper-core/src/runtime/environment/mod.rsdos2unix.git diff -w HEAD~1(against the post-#235 state) produces zero non-whitespace differences — the file content is byte-identical to the pre-#235 state plus the three small semantic refactors #235 actually intended.Test plan
git diff -w HEAD~1on each affected file → zero outputgit diff --ignore-all-space --ignore-blank-lines HEAD~1on each affected file → zero linescargo build -p shipper-core— cleanfile .gitattributes→ still LF (the file's own eol survived the edit)Why this works for future PRs
After this lands, future Edit operations on these files will store LF on commit even when the working tree is CRLF — the
.gitattributesrule makes git do the conversion at index time. No more line-ending churn in diffs.