Skip to content

refactor(esp8266): apply boards.txt menu props as pure config -> config rules - #1610

Merged
zackees merged 2 commits into
mainfrom
refactor/pure-esp8266-props
Oct 2, 2026
Merged

zackees merged 2 commits into
mainfrom
refactor/pure-esp8266-props

Conversation

@zackees

@zackees zackees commented Oct 2, 2026

Copy link
Copy Markdown
Member

Replaces the in-place apply_esp8266_board_props(&props, &mut config), which rewrote defines and linker libs through four scattered retain/push/overwrite blocks, with a side-effect-free for_board_props(config, &props) -> config. It is composed from these named rules:

  • with_sdk_define: replace the recipe's NONOSDK* key/value define with the board's sdk
  • board_define_flags + with_define: fold the -D tokens from flash_flags/lwip_flags/mmuflags/vtable_flags, last one wins
  • with_first_lib_replaced: the lwip_lib and stdcpp_lib variants

It is applied in one line:

let mcu_config = for_board_props(get_esp8266_config()?, &board_props);

Behaviour is unchanged. This path had no tests before; this PR adds tests for the none/no-op case and for the full rewrite.

Same theme as the #1605 review: fixups should be pure state -> state functions applied once.

Local checks: fmt, clippy -p fbuild-build-esp --all-targets -D warnings, and test -p fbuild-build-esp (146 passed). e2e: fbuild build <fastled>/.build/fbuild/esp8266 -e esp8266 --clean with this branch's binary and daemon: build succeeded (flash 269911 B).

…ig rules

`apply_esp8266_board_props(&props, &mut config)` rewrote defines and linker
libs in place through four scattered blocks. Replace it with
`for_board_props(config, &props) -> config`, composed from the named rules
`with_sdk_define`, `with_define` (folded over `board_define_flags`), and
`with_first_lib_replaced`, applied in one line where the MCU config loads.

Behaviour is unchanged; adds tests for the previously untested path.

Co-Authored-By: Claude <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 14 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository: FastLED/fbuild/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 848a946c-e00b-4993-b5ee-ea73b485a419

📥 Commits

Reviewing files that changed from the base of the PR and between 795ca58 and 71c909f.

📒 Files selected for processing (3)
  • crates/fbuild-build-esp/src/esp8266/board_props.rs
  • crates/fbuild-build-esp/src/esp8266/mod.rs
  • crates/fbuild-build-esp/src/esp8266/orchestrator.rs
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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.

Keeps orchestrator.rs under the 1000-line gate.

Co-Authored-By: Claude <noreply@anthropic.com>
@zackees
zackees merged commit 3e5aad7 into main Oct 2, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Triage

Development

Successfully merging this pull request may close these issues.

1 participant