Skip to content

fix(ci): unblock Formatting + Documentation on main#255

Merged
zackees merged 1 commit into
mainfrom
fix/fmt-and-rustdoc
May 23, 2026
Merged

fix(ci): unblock Formatting + Documentation on main#255
zackees merged 1 commit into
mainfrom
fix/fmt-and-rustdoc

Conversation

@zackees
Copy link
Copy Markdown
Member

@zackees zackees commented May 23, 2026

Summary

Two pre-existing failures on main since the loc-gate-refactor merge (#252) and the running-process-core 3.4 refactor (#254). Both are now blocking branch protection on every PR.

  • Formattingcargo fmt --all over the 16 files rustfmt flagged. Mechanical whitespace/trailing-comma adjustments; no semantic changes.
  • Documentationrustdoc::private-intra-doc-links (implied by -D warnings) rejects [name] doc-link syntax pointing at private modules. The LOC-gate split PRs created several new private submodules whose parent mod.rs files still link to them as if they were public. Demoted those links to backticked plain text (with a note that the submodules are private internals).

Files touched for Docs: fbuild-config/src/board/mod.rs, fbuild-deploy/src/esp32/mod.rs, fbuild-deploy/src/esp32_native/write.rs, fbuild-build/src/stm32/orchestrator/mod.rs, fbuild-core/src/{containment,subprocess}.rs.

Why not just make the modules pub

The LOC-gate splits intentionally kept the implementation submodules private and re-exported only the public types from the parent mod.rs. Making the submodules pub would expose internal layout. Plain-text references preserve the original intent.

Test plan

  • uv run soldr cargo fmt --all -- --check — clean
  • RUSTDOCFLAGS=-D warnings uv run soldr cargo doc --workspace --no-deps — clean (was 15 errors before)
  • CI: Formatting + Documentation both green

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Style

    • Consolidated and reformatted import statements across multiple modules for consistency.
    • Reformatted variable assignments and expressions throughout the codebase for improved readability.
  • Documentation

    • Updated module-level documentation comments to clarify submodule responsibilities and structure.
    • Enhanced function documentation with improved formatting and clarity.

Review Change Stack

Two pre-existing failures on main since the loc-gate-refactor merge
(#252) and the running-process-core 3.4 refactor (#254):

**Formatting** — `cargo fmt --all` over the 16 files the rustfmt CI
flagged. All changes are mechanical whitespace/trailing-comma
adjustments; no semantic changes.

**Documentation** — rustdoc `-D warnings` with
`-D rustdoc::private-intra-doc-links` rejects `[`name`]` doc-link
syntax that points at private modules. The LOC-gate split commits
(boards.rs, esp32 deploy, stm32 orchestrator) left intra-doc-link
references to their newly-private submodules, and my containment.rs +
subprocess.rs additions in #254 added two more. Demote the link
syntax to backticked plain text on:
- `crates/fbuild-config/src/board/mod.rs` (loaders, methods, db)
- `crates/fbuild-deploy/src/esp32/mod.rs` (verify, qemu, image, parse)
- `crates/fbuild-deploy/src/esp32_native/write.rs` (super::progress)
- `crates/fbuild-build/src/stm32/orchestrator/mod.rs`
  (arduino_mbed, framework_props, includes, variant_files)
- `crates/fbuild-core/src/{containment,subprocess}.rs`
  (windows_job::assign, ContainedProcessGroup::spawn_with_containment)

Tests: `cargo fmt --check` and
`RUSTDOCFLAGS=-D warnings cargo doc --workspace --no-deps` both green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 23, 2026

📝 Walkthrough

Walkthrough

This PR applies systematic code formatting across the fbuild repository: consolidating multi-line import statements and let-bindings to single lines, reordering imports for consistency, and updating documentation comments to clarify module structures and implementation details. No functional behavior or public APIs are modified.

Changes

Code Formatting and Documentation Refresh

Layer / File(s) Summary
Build orchestrator imports and let-binding formatting
crates/fbuild-build/src/esp32/orchestrator/build.rs, crates/fbuild-build/src/esp32/orchestrator/framework_libs.rs, crates/fbuild-build/src/esp32/orchestrator/local_libs.rs, crates/fbuild-build/src/stm32/orchestrator/mod.rs
Import statements reformatted to single lines and reordered; let-bindings for InstalledLibrary::new and apply_overlay_flags consolidated to single lines. STM32 orchestrator module documentation updated to clarify private submodule internals.
Core containment/subprocess documentation and daemon formatter
crates/fbuild-core/src/containment.rs, crates/fbuild-core/src/subprocess.rs, crates/fbuild-daemon/src/handlers/emulator/shared.rs
Documentation comments in containment and subprocess modules expanded to explain EPERM avoidance mechanisms and Windows job containment implementation details. QemuRunResult initializer in daemon handler reformatted for consistency.
Compile database test formatting
crates/fbuild-build/src/compile_database/tests/generate.rs, crates/fbuild-build/src/compile_database/tests/serialization_and_write.rs
Multi-line let-bindings for include_flags and build_content consolidated to single lines.
Daemon emulator test let-binding formatting
crates/fbuild-daemon/src/handlers/emulator/tests_outcome.rs, crates/fbuild-daemon/src/handlers/emulator/tests_process.rs
Emulator test let-bindings for outcome and system_root assignments consolidated from multi-line to single-line format.
ESP32 deployer documentation and import consolidation
crates/fbuild-deploy/src/esp32/mod.rs, crates/fbuild-deploy/src/esp32/qemu.rs, crates/fbuild-deploy/src/esp32/tests.rs
Module-level documentation for ESP32 deployer submodules reflowed and clarified; pub-use export list and import statements reformatted; checksum expression in tests consolidated to single line.
ESP32 native deployment formatting and docs
crates/fbuild-deploy/src/esp32_native/tests.rs, crates/fbuild-deploy/src/esp32_native/write.rs
Transport import list formatting adjusted for consistency; documentation reference to LoggingProgressBridge reformatted.
Deployer end-of-file formatting
crates/fbuild-deploy/src/esp32/deployer.rs
Minor closing-block line span adjustment.
Configuration board documentation and CLI dispatch imports
crates/fbuild-config/src/board/mod.rs, crates/fbuild-config/src/board/tests.rs, crates/fbuild-cli/src/cli/dispatch.rs
Board module documentation list reflowed to clarify submodule responsibilities; board test let-binding consolidated to single line; CLI dispatch import statement reordered.
Python daemon connection import formatting
crates/fbuild-python/src/daemon_connection.rs
Multi-line outcome import consolidated to single line.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

  • FastLED/fbuild#28: Refactored emulator runner and test infrastructure; this PR includes a minor formatting adjustment to the QemuRunResult initializer added in that work.

Poem

🐰 With whiskers twitched and paws held high,
We sorted imports, line by line,
And documented what's built inside—
The code flows cleaner, neat, and fine!
No logic changed, just style refined,
A formatting pass, organized and kind.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title clearly and specifically describes the primary fix: unblocking Formatting and Documentation checks on the main branch.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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-and-rustdoc

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 and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
crates/fbuild-core/src/containment.rs (1)

101-106: 💤 Low value

Optional: Consider standardizing doc comment style.

Lines 101-104 use //! (inner doc comments) while lines 105-106 use /// (outer doc comments), but all six lines document the spawn_contained function below. For consistency, consider using /// throughout this block since these are function-level docs, not module-level docs.

This inconsistency pre-dates this PR (lines 101-102 were not modified), so it's purely an optional cleanup.

♻️ Proposed consistency fix
-//! **Windows**: spawn directly, then assign the child handle to a Job
-//! Object with `JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE`
-//! (via the private `windows_job::assign` helper) — the same containment
-//! mechanism the pre-publication `running-process-core` rev used internally,
+/// **Windows**: spawn directly, then assign the child handle to a Job
+/// Object with `JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE`
+/// (via the private `windows_job::assign` helper) — the same containment
+/// mechanism the pre-publication `running-process-core` rev used internally,
 /// reimplemented locally since the published 3.4 API no longer exposes
 /// `spawn_with_containment(_, Containment::Contained)`.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/fbuild-core/src/containment.rs` around lines 101 - 106, The doc
comment block above the spawn_contained function mixes inner (//!) and outer
(///) doc comments; standardize them to outer doc comments for function-level
documentation by replacing the //! lines (lines documenting the same text as the
following ///) with /// so all six lines use /// and consistently document
spawn_contained.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@crates/fbuild-core/src/containment.rs`:
- Around line 101-106: The doc comment block above the spawn_contained function
mixes inner (//!) and outer (///) doc comments; standardize them to outer doc
comments for function-level documentation by replacing the //! lines (lines
documenting the same text as the following ///) with /// so all six lines use
/// and consistently document spawn_contained.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 631f8c64-7802-4c2f-a337-ac056bc678d0

📥 Commits

Reviewing files that changed from the base of the PR and between 951e4e6 and ec047a6.

📒 Files selected for processing (21)
  • crates/fbuild-build/src/compile_database/tests/generate.rs
  • crates/fbuild-build/src/compile_database/tests/serialization_and_write.rs
  • crates/fbuild-build/src/esp32/orchestrator/build.rs
  • crates/fbuild-build/src/esp32/orchestrator/framework_libs.rs
  • crates/fbuild-build/src/esp32/orchestrator/local_libs.rs
  • crates/fbuild-build/src/stm32/orchestrator/mod.rs
  • crates/fbuild-cli/src/cli/dispatch.rs
  • crates/fbuild-config/src/board/mod.rs
  • crates/fbuild-config/src/board/tests.rs
  • crates/fbuild-core/src/containment.rs
  • crates/fbuild-core/src/subprocess.rs
  • crates/fbuild-daemon/src/handlers/emulator/shared.rs
  • crates/fbuild-daemon/src/handlers/emulator/tests_outcome.rs
  • crates/fbuild-daemon/src/handlers/emulator/tests_process.rs
  • crates/fbuild-deploy/src/esp32/deployer.rs
  • crates/fbuild-deploy/src/esp32/mod.rs
  • crates/fbuild-deploy/src/esp32/qemu.rs
  • crates/fbuild-deploy/src/esp32/tests.rs
  • crates/fbuild-deploy/src/esp32_native/tests.rs
  • crates/fbuild-deploy/src/esp32_native/write.rs
  • crates/fbuild-python/src/daemon_connection.rs
💤 Files with no reviewable changes (2)
  • crates/fbuild-daemon/src/handlers/emulator/shared.rs
  • crates/fbuild-deploy/src/esp32/deployer.rs

@zackees zackees merged commit 6175424 into main May 23, 2026
84 of 87 checks passed
@zackees zackees mentioned this pull request May 23, 2026
4 tasks
zackees added a commit that referenced this pull request May 23, 2026
Patch release rolling up fixes and CI/dev-tooling cleanup since v2.2.3.

User-facing
-----------
- fix(teensy41): link `libarm_cortexM7lfsp_math` so Teensy Audio FFT
  examples (`Ports/PJRCSpectrumAnalyzer`) link cleanly (#258 / #257).
  First teensy41 CI green path since mid-April.

Internal / CI / dev tooling
---------------------------
- chore(deps): adopt upstream soldr/zccache/setup-soldr updates;
  `zccache-artifact` 1.4.0 -> 1.8; `zccache` PyPI >= 1.8.2;
  `profile = "minimal"` in rust-toolchain.toml; `prebuild-deps: none`
  workaround for the setup-soldr cargo-chef regression (#253).
- refactor(fbuild-core): migrate to `running-process-core` 3.4 from
  crates.io, dropping the abandoned-branch git pin (#254).
- fix(ci): unblock Formatting + Documentation on main after the
  loc-gate-refactor split (#255).
- chore(dev-tools): drop `soldr` from `ci/dev-tools/pyproject.toml`;
  require a globally-installed `soldr` everywhere; `uv run soldr ...`
  is now actively rejected by the tool_guard hook (#256 / #251).

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