Skip to content

fix(ci): cargo fmt + LOC gate fixes for #813 async migration regressions#825

Merged
zackees merged 1 commit into
mainfrom
fix/fmt-loc-async-813
Jun 29, 2026
Merged

fix(ci): cargo fmt + LOC gate fixes for #813 async migration regressions#825
zackees merged 1 commit into
mainfrom
fix/fmt-loc-async-813

Conversation

@zackees

@zackees zackees commented Jun 29, 2026

Copy link
Copy Markdown
Member

Summary

Two non-blocking-pre-merge regressions surfaced on main after #823 (full-async tokio migration) landed:

  • Formatting — 21 files needed cargo fmt after the wide-ranging async/.await edits.
  • LOC Gate (1000 LOC limit) — two files crossed the limit:
    • websockets.rs (1133 LOC) → extracted the inline #[cfg(test)] mod tests block to websockets_tests.rs via #[path] mod, dropping the main file to 853 LOC.
    • manager.rs (1003 LOC) → condensed the historical strategy comment to a one-liner pointer to docs/architecture/serial.md, dropping to 989 LOC.

LPC checksum-mismatch failures on main (LPCXpresso845-MAX, LPC804) are a pre-existing infra issue unrelated to #813 and are not addressed here.

Test plan

  • soldr cargo fmt --all -- --check (clean)
  • soldr cargo check --workspace --all-targets
  • soldr cargo test -p fbuild-daemon --lib (182 passed) — confirms the externalized tests module still picks up the same test set
  • wc -l confirms both touched files are under 1000

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Improved build handling for RP2040 targets by passing additional build context during orchestration.
  • Bug Fixes

    • Removed hidden encoding characters from several source files and cleaned up formatting across build, deploy, and linker flows.
    • Kept command execution, deployment, and archive behavior unchanged while improving consistency.
  • Tests

    • Moved WebSocket-related tests into a dedicated test module and added broader coverage for message coalescing and reader control behavior.
  • Documentation

    • Shortened and clarified serial manager documentation, with a pointer to the architecture guide.

The #813 full-async migration landed via admin-merge with two
non-blocking-pre-merge regressions:

1. **Formatting** — 21 files needed cargo fmt after the wide-ranging
   async/.await edits. Ran 'soldr cargo fmt --all'.

2. **LOC Gate (1000 LOC limit)** — two files crossed the limit:
   - websockets.rs (1133 LOC) — extracted the inline #[cfg(test)] mod
     tests block to websockets_tests.rs via #[path] mod, dropping the
     main file to 853 LOC.
   - manager.rs (1003 LOC) — condensed the historical strategy
     comment to a one-liner pointer to docs/architecture/serial.md,
     dropping to 989 LOC.

LPC checksum-mismatch failures on main (LPCXpresso845-MAX, LPC804)
are a pre-existing infra issue unrelated to #813 and are not addressed
here.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zackees zackees merged commit cda756f into main Jun 29, 2026
0 of 95 checks passed
@coderabbitai

coderabbitai Bot commented Jun 29, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: d3df8762-c22f-4573-8d58-9512d170f468

📥 Commits

Reviewing files that changed from the base of the PR and between 2667e28 and 2bd89a8.

📒 Files selected for processing (24)
  • crates/fbuild-build/src/generic_arm/arm_linker.rs
  • crates/fbuild-build/src/lib.rs
  • crates/fbuild-build/src/nrf52/nrf52_linker.rs
  • crates/fbuild-build/src/nrf52/orchestrator.rs
  • crates/fbuild-build/src/parallel.rs
  • crates/fbuild-build/src/renesas/orchestrator.rs
  • crates/fbuild-build/src/renesas/renesas_linker.rs
  • crates/fbuild-build/src/rp2040/orchestrator.rs
  • crates/fbuild-build/src/sam/orchestrator.rs
  • crates/fbuild-build/src/sam/sam_linker.rs
  • crates/fbuild-build/src/script_runtime_tests.rs
  • crates/fbuild-build/src/silabs/silabs_linker.rs
  • crates/fbuild-build/src/teensy/orchestrator.rs
  • crates/fbuild-build/src/teensy/teensy_linker.rs
  • crates/fbuild-core/src/response_file.rs
  • crates/fbuild-core/src/subprocess.rs
  • crates/fbuild-daemon/src/handlers/websockets.rs
  • crates/fbuild-daemon/src/handlers/websockets_tests.rs
  • crates/fbuild-deploy/src/esp32/deployer.rs
  • crates/fbuild-deploy/src/lib.rs
  • crates/fbuild-deploy/src/teensy/mod.rs
  • crates/fbuild-packages/src/library/arduino_api.rs
  • crates/fbuild-serial/src/crash_decoder.rs
  • crates/fbuild-serial/src/manager.rs

📝 Walkthrough

Walkthrough

BOM characters are removed from doc comment lines across multiple crates, and numerous call sites are reformatted (line wrapping, single-line consolidation) without logic changes. WebSocket tests are extracted from an inline module into a separate websockets_tests.rs file. The RP2040 orchestrator gains additional arguments in its run_sequential_build_with_libs call.

Changes

Formatting, BOM Removal, and Minor Functional Changes

Layer / File(s) Summary
WebSocket test extraction
crates/fbuild-daemon/src/handlers/websockets.rs, crates/fbuild-daemon/src/handlers/websockets_tests.rs
Inline mod tests block is replaced with a #[path = "websockets_tests.rs"] mod tests; declaration; the new file contains all relocated tests plus run_toy_reader, coalesce_for_test, and four test functions covering reader control and writer coalescing behavior.
RP2040 orchestrator pipeline call update
crates/fbuild-build/src/rp2040/orchestrator.rs
run_sequential_build_with_libs call gains TargetArchitecture::Arm, "RP2040" label, and start timestamp as additional arguments.
BOM removal and reformatting across crates
crates/fbuild-build/src/..., crates/fbuild-core/src/..., crates/fbuild-deploy/src/..., crates/fbuild-packages/src/..., crates/fbuild-serial/src/...
Leading BOM/hidden characters removed from doc comments in linker and orchestrator files; call sites reformatted across linkers, orchestrators, subprocess helpers, test files, and deployers with no logic changes. SharedSerialManager doc comment shortened to reference external architecture doc.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • FastLED/fbuild#13: Introduces the RP2040 orchestrator wiring that the modified run_sequential_build_with_libs call in this PR extends with new arguments.

Poem

🐇 Hoppity-hop through the files I go,
Sweeping out BOMs from each row,
Tests get their own cozy .rs lair,
And RP2040 gets args to spare—
A tidy warren, line by line!

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/fmt-loc-async-813

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@zackees zackees deleted the fix/fmt-loc-async-813 branch June 29, 2026 15:40
zackees added a commit that referenced this pull request Jun 29, 2026
Release the #813 full-async tokio runtime migration. The work landed
across PR #823 (the migration itself, admin-merged), #824 (the local
Docker Linux verify infra), and #825 (cargo fmt + LOC gate fixes for
the migration's wide-ranging edits).

This is the first release on the unified async runtime — every
per-TU compile, every per-platform deploy, every subprocess
invocation now drives through the daemon's single tokio runtime,
making tokio-console visibility complete and removing the rayon /
std::thread::scope hybrid that the migration replaced.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant