ci: lint all OS targets on one Linux builder - #1526
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: FastLED/fbuild/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe Dylint workflow runs Linux checks and adds Windows and macOS checks for full runs. It configures compiled-library caching and skips library UI tests when Dylint sources are unchanged. CI tests assert the workflow settings. ChangesDylint workflow
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant BoundaryBaseline
participant Dylint
participant SuccessMarker
GitHubActions->>BoundaryBaseline: Read cross-target package selections
GitHubActions->>Dylint: Run workspace lint for enabled targets
Dylint-->>GitHubActions: Return lint results
GitHubActions->>BoundaryBaseline: Validate each target's observed ledger
GitHubActions->>SuccessMarker: Verify and normalize after successful checks
Merge Risk: ⚪ Minimal · up to No actionable issue is established for this change; it is mergeable after normal checks. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The workflow adds useful Windows and macOS lint coverage while retaining a read-only repository token and a fail-closed gate. The cross-target checks cover a selected set of crates rather than every crate, and compiled lint libraries are reused across commits, so the limits of those controls merit review. No introduced security vulnerability was established. Retained concerns
Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
4da8dc1 to
d2fe3f6
Compare
Change
Keep ordinary PR Dylint on one Linux runner, and make the existing
ci-fullDylint call and manual dispatch check Linux, Windows MSVC, and macOS targets from that same Linux builder. The full run prepares the catalogued prebuilt driver once, installs the pinned nightly's cross-targetrust-std, runs all 27 custom lints over the Linux workspace, then checks the five crates named by the platform-boundary baseline for Windows/macOS. Each target's actual Dylint observations are compared with the source ledger. UI fixtures run whendylints/changes, without rebuilding their separate test-profile trees on unrelated PRs.The compiled lint-library tree uses an
actions/cachekey based ondylints/**and the pinned nightly, so workflow-only and application-only commits can reuse it across source SHAs. A short/qualified nightly alias lets Soldr and the cache share one directory. The large target-specific workspace tree is outside this cache.This keeps the routine gate fast after #1524 made it Linux-host only, while restoring an explicit full-platform Dylint path. zackees/soldr#3426 tracks automatic target
rust-stdmaterialization; fbuild#1527 tracks removal of the temporary workflow install step after that Soldr fix ships.Evidence
fbuild-dylint-libraries-v1-linux-nightly-2026-05-28-....ban_manual_slash_normalizeDylint checks passed for Windows MSVC and macOS; fullsoldr cargo check --workspace --all-targetspassed for Windows GNU, Windows MSVC, and macOS. Workflow contract tests: 11 passed.Summary by CodeRabbit