Skip to content

fix(apply): use correct condition to skip activation#134

Merged
water-sucks merged 1 commit intonix-community:mainfrom
water-sucks:fix-stc-test
Nov 20, 2025
Merged

fix(apply): use correct condition to skip activation#134
water-sucks merged 1 commit intonix-community:mainfrom
water-sucks:fix-stc-test

Conversation

@water-sucks
Copy link
Copy Markdown
Collaborator

@water-sucks water-sucks commented Nov 20, 2025 •
edited by coderabbitai bot
Loading

Testing generations using nixos apply --no-boot was skipping activation entirely due to a broken condition.

This resolves that broken condition by only skipping running switch-to-configuration if BOTH --no-activate and --no-boot are specified.

Summary by CodeRabbit

  • Bug Fixes
    • Fixed the system activation logic to trigger correctly under the appropriate flag combinations. Previously, activation required both specific flags to be unset; now it occurs when at least one is not set, providing more flexible control over build behavior.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Nov 20, 2025 •
edited
Loading

Walkthrough

A logical operator in the activation condition for SystemBuild is changed from AND to OR in cmd/apply/apply.go. This alters whether the activation step executes based on NoActivate and NoBoot flag combinations.

Changes

Cohort / File(s) Summary
Activation condition logic
cmd/apply/apply.go		 Changed the Activate condition from !opts.NoActivate && !opts.NoBoot to !opts.NoActivate || !opts.NoBoot, switching the logic operator from AND to OR

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Areas requiring attention:

  • Verify the operator change is intentional and not a bug introduced by this PR
  • Confirm the behavior across all flag combinations: both flags false, one flag true/one false, both flags true
  • Ensure this change aligns with the intended activation semantics for system builds

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately reflects the main change: fixing a condition to skip activation in the apply command, which aligns with the changeset's core modification of the activation logic.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9dbe106 and e321c99.

📒 Files selected for processing (1)
  • cmd/apply/apply.go (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
cmd/apply/apply.go (1)
internal/configuration/configuration.go (1)
  • SystemBuild (75-77)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Build/Test
🔇 Additional comments (1)
cmd/apply/apply.go (1)

238-238: LGTM! The fix correctly resolves the activation skip bug.

The change from && to || properly implements the intended behavior: activation should only be skipped when BOTH --no-activate AND --no-boot are specified.

With the previous condition, using --no-boot alone would incorrectly prevent activation entirely. The new condition correctly allows:

  • --no-boot alone → runs "test" action (activates without creating boot entry)
  • --no-activate alone → runs "boot" action (creates boot entry without immediate activation)
  • Both flags together → skips activation (returns early at line 491)

The logic is consistent with the validation at lines 64-72 and the early return at lines 490-492.

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

  • Provide your own instructions using the high_level_summary_instructions setting.
  • Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
  • Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

  1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
  2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
  3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
  4. 📊 Contributor Summary — Include a Markdown table showing contributions:
    | Contributor | Lines Added | Lines Removed | Files Changed |
  5. ✔️ Additional Notes — Add any extra reviewer context.
    Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.

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.

@water-sucks water-sucks merged commit 64f7329 into nix-community:main Nov 20, 2025
2 checks passed
@water-sucks water-sucks deleted the fix-stc-test branch November 20, 2025 04:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@water-sucks