Skip to content

refactor(nrf52): apply the GCC 7 registry toolchain fixup as a pure function - #1608

Merged
zackees merged 1 commit into
mainfrom
refactor/pure-nrf52-fixup
Oct 2, 2026
Merged

zackees merged 1 commit into
mainfrom
refactor/pure-nrf52-fixup

Conversation

@zackees

@zackees zackees commented Oct 2, 2026

Copy link
Copy Markdown
Member

Replaces the in-place align_nrf52_flags_with_registry_toolchain(&mut config, version) with a side-effect-free for_registry_toolchain(config, version) -> config, composed from the named rules is_gcc7_registry_toolchain and without_release_lto. It is applied in one line at load time:

let mcu_config = for_registry_toolchain(get_nrf52_config_for_mcu(&mcu_lower)?, &toolchain_version);

Behaviour is unchanged: on GCC 7.2 (1.70201.*), only the release profile drops -flto/-fno-fat-lto-objects (compile) and -flto/-fuse-linker-plugin (link). The test now also asserts that the default toolchain leaves the recipe untouched and that the other profiles are kept.

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

Local checks: fmt, clippy -p fbuild-build-arm --all-targets -D warnings, and test -p fbuild-build-arm nrf52 (23 passed).

…unction

`align_nrf52_flags_with_registry_toolchain(&mut config, version)` mutated
the release profile in place. Replace it with
`for_registry_toolchain(config, version) -> config`, built from the named
rules `is_gcc7_registry_toolchain` and `without_release_lto`, and apply it
in one line where the MCU config is loaded.

Behaviour is unchanged.

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 28 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: a1b3ccda-7e32-4d43-89a9-937b3107e3d5

📥 Commits

Reviewing files that changed from the base of the PR and between 795ca58 and 2feba72.

📒 Files selected for processing (1)
  • crates/fbuild-build-arm/src/nrf52/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.

@zackees
zackees merged commit 0def986 into main Oct 2, 2026
13 checks passed
@fastled-project-sync fastled-project-sync Bot moved this to Triage in FastLED Tracker Oct 3, 2026
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