Skip to content

Conversation

veeso
Copy link
Member

@veeso veeso commented Sep 5, 2025

Summary by CodeRabbit

  • Refactor
    • Scoped lp-tool installation from system-wide to the Nomad client environment.
  • Impact
    • lp-tool remains available for Nomad workloads but may no longer be accessible globally on the host.
  • Chores
    • Reduced base system package footprint for cleaner, more targeted installs.

Copy link

cocogitto-bot bot commented Sep 5, 2025

✔️ 5b7e5f3 - Conventional commits check succeeded.

Copy link

coderabbitai bot commented Sep 5, 2025

Walkthrough

Moved lp-tool installation from global environment.systemPackages to services.nomad.extraPackages within base-install.nix, keeping the package installed via the Nomad client instead of system-wide.

Changes

Cohort / File(s) Summary of changes
Nomad package relocation
base-install.nix
Removed inputs.wind-tunnel.packages.x86_64-linux.lp-tool from environment.systemPackages; added the same package to services.nomad.extraPackages.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • cdunster
✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch install-lp-tool

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.

@veeso veeso marked this pull request as ready for review September 6, 2025 09:25
@veeso veeso requested a review from cdunster September 6, 2025 09:25
Copy link

@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.

Actionable comments posted: 0

🧹 Nitpick comments (1)
base-install.nix (1)

121-121: Optional: avoid hard-coding the architecture.

Makes the module reusable on non-x86_64 systems (if needed in future).

Apply:

-          inputs.wind-tunnel.packages.x86_64-linux.lp-tool
+          (builtins.getAttr "lp-tool" (builtins.getAttr pkgs.system inputs.wind-tunnel.packages))
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ff2bc21 and 5b7e5f3.

📒 Files selected for processing (1)
  • base-install.nix (1 hunks)
🔇 Additional comments (2)
base-install.nix (2)

121-121: Good move: install lp-tool via Nomad extraPackages (not system-wide).

This aligns the tool’s lifecycle with the Nomad client and keeps the base system lean.


109-122: Verified: No system-wide lp-tool installations present. lp-tool only appears in extraPackages (base-install.nix:121) as intended.

@veeso veeso merged commit 59d2a97 into main Sep 8, 2025
5 of 6 checks passed
@veeso veeso deleted the install-lp-tool branch September 8, 2025 08:09
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.

2 participants