Conversation
📝 WalkthroughSummary by CodeRabbit
Summary by CodeRabbit
WalkthroughThis update performs a comprehensive refactor and namespace reorganization across the NixOS and Home Manager module hierarchy. It migrates module option paths from Changes
Sequence Diagram(s)sequenceDiagram
participant UserConfig
participant HomeManager
participant SystemModules
participant Packages
UserConfig->>HomeManager: Specify options (e.g., games, browser.clients, dev.tools)
HomeManager->>SystemModules: Map options to package lists via dynamic mappings
SystemModules->>Packages: Aggregate enabled package lists (deduped)
Packages-->>HomeManager: Provide final package set
HomeManager-->>UserConfig: Apply configuration (home.packages, session variables, etc.)
sequenceDiagram
participant UserConfig
participant DiscordModule
participant Packages
UserConfig->>DiscordModule: Set clients/overlays/rpc options
DiscordModule->>Packages: Map selected options to package lists
Packages-->>DiscordModule: Return package list
DiscordModule-->>UserConfig: Add packages to home.packages if any selected
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
💤 Files with no reviewable changes (1)
🔇 Additional comments (8)
✨ Finishing Touches🧪 Generate Unit Tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This PR refactors and enhances the Nix configuration modules by renaming the common namespace to hm, introducing reusable mappings for various package categories, and simplifying package selection logic via lib.filter and lib.concatMap. It also removes redundant settings and consolidates game modules into a single file.
- Renamed all configuration paths from
config.modules.commontoconfig.modules.hm - Introduced
*ToPackagemaps for editors, browsers, CLI tools, and mail services - Simplified home package assembly logic and consolidated game modules
Reviewed Changes
Copilot reviewed 82 out of 82 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| modules/hm/common/documentation/default.nix | Renamed config path, added editor-package map, simplified package list |
| modules/hm/common/dev/rust.nix | Renamed config path for Rust dev |
| modules/hm/common/dev/python.nix | Renamed config path for Python dev |
| modules/hm/common/dev/node/prisma.nix | Renamed config path for Prisma |
| modules/hm/common/dev/node/pm2.nix | Renamed config path for PM2 |
| modules/hm/common/dev/node/node.nix | Renamed config path for Node, kept package logic |
| modules/hm/common/dev/global-tools/nix.nix | Renamed config path for Nix global tools |
| modules/hm/common/dev/global-tools/cli.nix | Added CLI-to-package map, simplified package selection |
| modules/hm/common/dev/global-tools/act-github.nix | Renamed config path for act-github |
| modules/hm/common/dev/editor/vs-code.nix | Renamed config path for VS Code |
| modules/hm/common/dev/editor/jetbrains.nix | Renamed config path for JetBrains IDEs |
| modules/hm/common/dev/editor/dbeaver.nix | Renamed config path for DBeaver |
| modules/hm/common/dev/editor/android-studio.nix | Renamed config path for Android Studio |
| modules/hm/common/dev/dotnet.nix | Renamed config path for .NET, updated sdk-versions option |
| modules/hm/common/default.nix | Consolidated individual game modules into games.nix |
| modules/hm/common/communication/teams.nix | Renamed config path for Teams |
| modules/hm/common/communication/mail/default.nix | Renamed config path, expanded mail-service mapping |
| modules/hm/common/communication/mail/bluemail.nix | Removed unused cfg binding in bluemail module |
| modules/hm/common/browser/default.nix | Renamed config path, added browser & driver maps, simplified packages |
| TODO.md | Updated task status markers |
Comments suppressed due to low confidence (2)
modules/hm/common/dev/dotnet.nix:6
- Accessing an attribute with a hyphen using
cfg.sdk-versionsis interpreted as subtraction. Use quoted accesscfg."sdk-versions"or rename the option to a hyphen-free key.
sdkVersions = cfg.sdk-versions;
modules/hm/common/browser/default.nix:30
- The code references
cfg.driver.enable, but nodriveroption is declared underoptions.modules.hm.browser. Define adriver.enableoption or guard this reference to avoid an evaluation error.
drivers = lib.optionals cfg.driver.enable (
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 36
🔭 Outside diff range comments (17)
modules/system/common/hardware/autologin.nix (1)
11-23: 🛠️ Refactor suggestionMake
userandsessionmandatory & self-documentingBoth options default to
nullwhen nodefaultis supplied.
Because they are dereferenced inside thecfg.enablebranch, enabling the module without explicitly setting them will generate an invalid SDDM config ("null.desktop"/"null").- user = lib.mkOption { - type = lib.types.str; - description = "User to autologin"; - }; + user = lib.mkOption { + type = lib.types.nonEmptyStr; + description = "User to autologin"; + example = "alice"; + # No default → value is required when cfg.enable = true + }; - session = lib.mkOption { - type = lib.types.str; - description = "Session to autologin"; - }; + session = lib.mkOption { + type = lib.types.nonEmptyStr; + description = "Desktop session (without .desktop suffix)"; + example = "plasma"; + };Optionally, add an assertion to guarantee the suffix isn’t provided twice:
assertions = [{ assertion = !lib.hasSuffix ".desktop" cfg.session; message = "modules.system.hardware.autologin.session must be given without the .desktop suffix"; }];modules/system/common/hardware/boot.nix (1)
20-24:⚠️ Potential issueUse
lib.mkForce, notpkgs.lib.mkForce
pkgs.libis deprecated in recent Nixpkgs; the canonical accessor is justlib.
The current line will still work but ties the expression to the host’s package set and can break ifpkgsis overridden.- loader.systemd-boot.enable = pkgs.lib.mkForce false; + loader.systemd-boot.enable = lib.mkForce false;modules/hm/common/dev/dotnet.nix (1)
4-13:⚠️ Potential issueDynamic attribute access is syntactically invalid – Nix will fail to evaluate.
pkgs ? ${pkgName}andpkgs.${pkgName}cannot interpolate a variable in an attr-path.
Use a quoted string inside the attr-path instead.- (map (pkgName: - if pkgs ? ${pkgName} - then pkgs.${pkgName} - else throw "Package '${pkgName}' not found in pkgs" + (map (pkgName: + if pkgs ? "${pkgName}" + then pkgs."${pkgName}" + else throw "Package '${pkgName}' not found in pkgs" ) cfg.extraPackages);Failing to fix this breaks
home-managerevaluation as soon asextraPackagesis non-empty.modules/hm/common/shell/fzf.nix (1)
4-13: 🛠️ Refactor suggestionFeature flag is ineffective –
fzfis installed even when the module is disabled
home.packagesis appended unconditionally. This defeats the purpose of theenableoption introduced on line 8; users who explicitly disable the module will still get the package.- config.home.packages = (with pkgs; [ - fzf - ]); + config = lib.mkIf cfg.enable { + home.packages = (with pkgs; [ fzf ]); + };Applying the guard also aligns this module with the enable-gated pattern used elsewhere in the codebase (e.g.
obs.nix).modules/hm/common/extra/ignore-file-retriever.nix (1)
138-148: 🛠️ Refactor suggestionQuote
$watchDirsto avoid word-splitting when paths contain spaces
watchDirsis expanded unquoted inside theinotifywaitcommand.
If any watched directory has a space the service will break because bash will
split the value into multiple arguments.- watchDirs = lib.concatStringsSep " " ( - ["%h"] ++ map expandPath cfg.watchPaths - ); + watchDirs = lib.concatStringsSep " " ( + ["%h"] ++ map (p: lib.escapeShellArg (expandPath p)) cfg.watchPaths + );and later:
- "${pkgs.inotify-tools}/bin/inotifywait -m -r -e modify,move,create,delete \ - --format '%w%f' ${watchDirs} | \ + "${pkgs.inotify-tools}/bin/inotifywait -m -r -e modify,move,create,delete \ + --format '%w%f' ${watchDirs} | \(The variable is now already shell-escaped.)
modules/hm/common/communication/teams.nix (1)
12-14: 🧹 Nitpick (assertive)Consider switching to the official Microsoft build
teams-for-linuxis no longer actively maintained and has lagged behind the PWA
recommended by Microsoft.pkgs.teams(electron) or a PWA wrapper might give a
better UX and quicker updates.modules/system/common/nix/linux-cachyos.nix (1)
24-30: 🧹 Nitpick (assertive)Optional: co-locate substituter key with URL
Minor readability nit – co-locating the public key directly beneath its
matching substituter makes maintenance easier when multiple substituters are
present.modules/hm/common/dev/python.nix (1)
79-82: 🛠️ Refactor suggestionGuard against empty
versionslist
builtins.head cfg.versionsthrows if the list is empty. Either validate that
versionsis non-empty or fall back gracefully.- default = builtins.head cfg.versions; + default = if cfg.versions != [] then builtins.head cfg.versions + else lib.mkDefault "312";modules/system/common/backup/syncthing.nix (1)
35-43:⚠️ Potential issue
lib.cleanSourcewill copy user data into the Nix store – remove it
lib.cleanSourceis intended for build-time sources; applying it to a runtime path like${cfg.dirSync}/${cfg.subDir}will try to import the whole directory (default “/”) into the store, which is huge and evaluation-time illegal.- dataDir = lib.cleanSource "${cfg.dirSync}/${cfg.subDir}"; # default location for new folders + # Use the path directly; no need to copy it to the Nix store + dataDir = "${cfg.dirSync}/${cfg.subDir}";This prevents accidental mass copying and keeps the module evaluation fast and safe.
modules/system/common/dev/dev.nix (1)
9-34: 🧹 Nitpick (assertive)Looks good – maybe allow consumers to extend
nix-ldlibrariesThe rename to
modules.system.dev.dev-globalis correct.
For future extensibility, exposing thelibrarieslist as an option (with a sensible default) would let users add project-specific shared objects without forking the module.(Not blocking for this PR.)
modules/hm/common/engine/default.nix (1)
15-21:⚠️ Potential issueOption should live under
modules.hm.engine, not at the root
Declaring:options.modules.hm = { engine = …; };mixes values with sub-trees and may collide with other keys (
dev,shell, …). Nest the option:-options.modules.hm = { - engine = … -}; +options.modules.hm.engine = lib.mkOption { … };This keeps the namespace flat and predictable.
modules/hm/common/dev/node/pm2.nix (1)
12-14:⚠️ Potential issue
pm2attribute likely missing – usenodePackages.pm2.In current
nixpkgsthe PM2 CLI lives underpkgs.nodePackages.pm2; there is no top-levelpkgs.pm2.
Using the wrong attribute will break evaluation.- home.packages = (with pkgs; [ - pm2 - ]); + home.packages = (with pkgs; [ + nodePackages.pm2 + ]);modules/hm/common/dev/node/prisma.nix (1)
12-15:⚠️ Potential issueWrong package attribute –
prismais not top-level.
nixpkgsexports the CLI aspkgs.nodePackages.prisma(or, for engines,prisma-engines).
Referencingpkgs.prismawill fail at evaluation time.- home.packages = (with pkgs; [ - prisma - ]); + home.packages = (with pkgs; [ + nodePackages.prisma + ]);modules/hm/common/dev/editor/android-studio.nix (1)
12-15:⚠️ Potential issue
android-studio-toolsattribute does not exist – replace withandroid-tools.
nixpkgsprovidesandroid-tools(adb/fastboot), notandroid-studio-tools.
Leaving the current name will break the build.- android-studio - android-studio-tools + android-studio + android-toolsmodules/system/hosts/fern/wol.nix (2)
23-27:⚠️ Potential issueDynamic interface key needs quoting – current syntax is invalid Nix.
Attribute paths cannot interpolate variables without quotes.
Evaluation will fail oninterfaces.${cfg.interface}.- interfaces.${cfg.interface} = { + interfaces."${cfg.interface}" = { wakeOnLan.enable = true; };
30-48: 🧹 Nitpick (assertive)Optional: unify
ExecStart/ExecStop.Both commands set WoL to
g;ExecStopcan be dropped or replaced with a true no-op to shorten the service definition.modules/hm/common/emulator/default.nix (1)
76-80: 🧹 Nitpick (assertive)
WINEARCHhard-coded towin64Some users still rely on 32-bit prefixes. Consider exposing an option
or inferring it from the chosen Wine package.- WINEARCH = "win64"; + WINEARCH = cfg.wine.arch or "win64";(where
wine.archis a newenum ["win64" "win32"]).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (82)
TODO.md(1 hunks)modules/hm/common/browser/default.nix(2 hunks)modules/hm/common/communication/mail/bluemail.nix(0 hunks)modules/hm/common/communication/mail/default.nix(1 hunks)modules/hm/common/communication/teams.nix(1 hunks)modules/hm/common/default.nix(1 hunks)modules/hm/common/dev/dotnet.nix(2 hunks)modules/hm/common/dev/editor/android-studio.nix(1 hunks)modules/hm/common/dev/editor/dbeaver.nix(1 hunks)modules/hm/common/dev/editor/jetbrains.nix(1 hunks)modules/hm/common/dev/editor/vs-code.nix(1 hunks)modules/hm/common/dev/global-tools/act-github.nix(1 hunks)modules/hm/common/dev/global-tools/cli.nix(1 hunks)modules/hm/common/dev/global-tools/nix.nix(1 hunks)modules/hm/common/dev/node/node.nix(2 hunks)modules/hm/common/dev/node/pm2.nix(1 hunks)modules/hm/common/dev/node/prisma.nix(1 hunks)modules/hm/common/dev/python.nix(2 hunks)modules/hm/common/dev/rust.nix(1 hunks)modules/hm/common/documentation/default.nix(1 hunks)modules/hm/common/documentation/obsidian.nix(2 hunks)modules/hm/common/emulator/default.nix(3 hunks)modules/hm/common/engine/default.nix(2 hunks)modules/hm/common/extra/ignore-file-retriever.nix(2 hunks)modules/hm/common/games/games.nix(1 hunks)modules/hm/common/games/joystick.nix(1 hunks)modules/hm/common/games/mangohud.nix(1 hunks)modules/hm/common/games/minecraft.nix(0 hunks)modules/hm/common/games/northstar.nix(0 hunks)modules/hm/common/games/roblox.nix(0 hunks)modules/hm/common/games/rocket-league.nix(0 hunks)modules/hm/common/games/star-citizen.nix(0 hunks)modules/hm/common/multimedia/gimp.nix(1 hunks)modules/hm/common/multimedia/mpv.nix(1 hunks)modules/hm/common/multimedia/obs.nix(1 hunks)modules/hm/common/multimedia/openshot-qt.nix(1 hunks)modules/hm/common/multimedia/parsec.nix(1 hunks)modules/hm/common/multimedia/stremio.nix(1 hunks)modules/hm/common/network/tunnel.nix(3 hunks)modules/hm/common/shell/btop.nix(1 hunks)modules/hm/common/shell/disk-usage.nix(1 hunks)modules/hm/common/shell/fzf.nix(1 hunks)modules/hm/common/shell/navi.nix(1 hunks)modules/hm/common/shell/ranger.nix(1 hunks)modules/hm/common/shell/starship.nix(1 hunks)modules/hm/common/shell/tools.nix(1 hunks)modules/hm/common/utilities/filezilla.nix(1 hunks)modules/hm/common/utilities/kde-connect.nix(1 hunks)modules/hm/common/utilities/scalar.nix(2 hunks)modules/hm/common/utilities/stacer.nix(1 hunks)modules/hm/desktops/configHydenix.nix(1 hunks)modules/hm/desktops/hydenix.nix(3 hunks)modules/hm/hosts/oak/default.nix(4 hunks)modules/system/common/backup/syncthing.nix(1 hunks)modules/system/common/default.nix(2 hunks)modules/system/common/dev/dev.nix(1 hunks)modules/system/common/dev/flatpak.nix(1 hunks)modules/system/common/dev/flutter.nix(2 hunks)modules/system/common/dev/php.nix(1 hunks)modules/system/common/games/games.nix(3 hunks)modules/system/common/games/gamescope.nix(1 hunks)modules/system/common/hardware/autologin.nix(1 hunks)modules/system/common/hardware/boot.nix(1 hunks)modules/system/common/hardware/powersave.nix(2 hunks)modules/system/common/networks/portmaster.nix(0 hunks)modules/system/common/networks/print/print.nix(1 hunks)modules/system/common/networks/vpn.nix(1 hunks)modules/system/common/networks/vpn/tailscale.nix(0 hunks)modules/system/common/nix/linux-cachyos.nix(1 hunks)modules/system/common/nix/nix-garbage.nix(1 hunks)modules/system/common/nix/polkit.nix(1 hunks)modules/system/common/security/antivirus.nix(1 hunks)modules/system/common/security/password-manager.nix(1 hunks)modules/system/common/virtualisation/containers.nix(1 hunks)modules/system/hosts/fern/default.nix(1 hunks)modules/system/hosts/fern/drivers.nix(2 hunks)modules/system/hosts/fern/openrgb.nix(0 hunks)modules/system/hosts/fern/plex.nix(1 hunks)modules/system/hosts/fern/sunshine.nix(1 hunks)modules/system/hosts/fern/vfio/default.nix(2 hunks)modules/system/hosts/fern/wol.nix(1 hunks)modules/system/hosts/oak/default.nix(1 hunks)
💤 Files with no reviewable changes (9)
- modules/hm/common/communication/mail/bluemail.nix
- modules/hm/common/games/northstar.nix
- modules/hm/common/games/star-citizen.nix
- modules/hm/common/games/roblox.nix
- modules/hm/common/games/minecraft.nix
- modules/system/common/networks/vpn/tailscale.nix
- modules/hm/common/games/rocket-league.nix
- modules/system/common/networks/portmaster.nix
- modules/system/hosts/fern/openrgb.nix
🔇 Additional comments (49)
modules/hm/common/utilities/stacer.nix (1)
4-8:Details
✅ Verification successful
Namespace migration looks correct — check for dangling references
cfgand theoptionsblock now point tomodules.hm.utilities.stacer, matching the new convention.
Please scan the codebase for any leftovermodules.common.utilities.stacerreferences to avoid silent mis-configuration at runtime.
🏁 Script executed:
#!/usr/bin/env bash # Find stale references to the old namespace. rg -n $'modules\\.common\\.utilities\\.stacer'Length of output: 44
Let's broaden the search to catch any stray references under
common.utilities.stacer:
🏁 Script executed:
#!/usr/bin/env bash # Double-check for any leftover references to the old namespace rg -n 'common\\.utilities\\.stacer'Length of output: 37
No stale references to the old namespace detected
A repository-wide search for bothmodules.common.utilities.stacerandcommon.utilities.stacerreturned no matches. The namespace migration is complete.modules/hm/common/utilities/filezilla.nix (1)
4-5:Details
✅ Verification successful
Verify that all call-sites were migrated to the new namespace
cfg = config.modules.hm.utilities.filezillaswitches from the oldmodules.common.*path. Any stale references will silently disable this module.Run a quick grep to be safe:
🏁 Script executed:
#!/usr/bin/env bash # Look for forgotten usages of the old namespace rg -n --no-heading $'modules\\.common\\.utilities\\.filezilla'Length of output: 60
🏁 Script executed:
#!/usr/bin/env bash # Double‐check for any lingering old namespace references with the config prefix rg -n --no-heading 'config\.modules\.common\.utilities\.filezilla'Length of output: 68
No stale
modules.common.utilities.filezillareferences remainI ran a grep for both
rg -n --no-heading 'modules\.common\.utilities\.filezilla' rg -n --no-heading 'config\.modules\.common\.utilities\.filezilla'and found zero matches. All call-sites have been successfully migrated to
config.modules.hm.utilities.filezilla.modules/system/common/hardware/autologin.nix (1)
7-9:Details
✅ Verification successful
Ensure all callers were migrated to the new namespace
cfgnow points toconfig.modules.system.hardware.autologin.
Any leftover references to the formerconfig.modules.hardware.autologinpath will silently evaluate tonulland break autologin at runtime.Run the following to confirm that no files still use the old namespace:
🏁 Script executed:
#!/bin/bash # Find occurrences of the outdated path. rg -n "modules[.]hardware[.]autologin" --glob '*.nix'Length of output: 55
🏁 Script executed:
#!/bin/bash # Re-verify references to the old autologin namespace, including the config prefix. rg -n "modules\\.hardware\\.autologin" --glob '*.nix' rg -n "config\\.modules\\.hardware\\.autologin" --glob '*.nix'Length of output: 115
All autologin references updated to the new namespace
A full search for
config.modules.hardware.autologinin.nixfiles returned no matches. The migration toconfig.modules.system.hardware.autologinis complete—no further changes are needed.modules/system/common/nix/nix-garbage.nix (1)
4-7:Details
✅ Verification successful
Double-check for stale references in the tree
Please grep the repo for
modules.nix.nix-garbageto ensure no callers still rely on the old path.
🏁 Script executed:
#!/usr/bin/env bash # Find any lingering references to the old garbage-collector path rg --line-number --color=always $'modules\\.nix\\.nix-garbage'Length of output: 61
No stale references to
modules.system.nix.nix-garbagefound
Ran the ripgrep search below and confirmed there are no lingering callers of the old path:rg --line-number 'modules\.nix\.nix-garbage'No matches were detected—this change can be considered safe.
modules/hm/common/dev/editor/dbeaver.nix (1)
4-8: HM namespace switch LGTM – verify consumersThe switch to
modules.hm.dev.editor.dbeaveris consistent with the broader refactor. Make sure every host/user configuration that previously setmodules.common.dev.editor.dbeaver.enableis updated, otherwise the module will silently disable.modules/hm/common/dev/global-tools/nix.nix (1)
4-8:Details
✅ Verification successful
Path realignment OK – run a quick repo-wide sweep
The new
modules.hm.dev.global-tools.nixpath compiles, but a stray reference to the oldmodules.common.dev.global-tools.nixwill break enablement.
🏁 Script executed:
#!/usr/bin/env bash rg --line-number --color=always $'modules\\.common\\.dev\\.global-tools\\.nix'Length of output: 75
No stray references to the old path found
A repo-wide sweep withrg --line-number --color=always 'modules\.common\.dev\.global-tools\.nix'returned no matches, confirming the path realignment is complete.
modules/hm/common/shell/btop.nix (1)
4-8: Shell module namespace updated – goodChange is straightforward and consistent. No functional issues spotted.
modules/hm/common/multimedia/stremio.nix (1)
4-8:Details
✅ Verification successful
Namespace refactor looks correct – please sweep for lingering callers
cfgand the option path are migrated tomodules.hm.multimedia.stremio, matching the new convention.
Please run a quick repo-wide search to ensure no module or overlay still references the oldmodules.common.multimedia.stremiopath; stale references will silently disable the option.
🏁 Script executed:
#!/usr/bin/env bash # Find any leftover usages of the old namespace rg -n $'modules\\.common\\.multimedia\\.stremio'Length of output: 46
Namespace refactor verified – no lingering callers found
I ran a repo-wide search for the oldmodules.common.multimedia.stremiopath and found no matches. The namespace has been fully migrated tomodules.hm.multimedia.stremio, so no further action is needed.
- No occurrences of
modules.common.multimedia.stremioin the codebase.modules/hm/common/multimedia/gimp.nix (1)
4-8:Details
❓ Verification inconclusive
Consistent rename applied – double-check downstream references
The switch to
modules.hm.multimedia.gimpmirrors the rest of the multimedia refactor.
Same verification concern as above: confirm that no file, README, or example configuration still uses the old path.
🏁 Script executed:
#!/usr/bin/env bash rg -n $'modules\\.common\\.multimedia\\.gimp'Length of output: 43
#!/usr/bin/env bash
Search for any lingering references to the old path in code, docs, or examples
rg -n 'modules.common.multimedia.gimp|common.multimedia.gimp'
Consistent rename applied – verify no leftover references
I searched for
modules.common.multimedia.gimpand found no matches. To ensure nothing slipped through, please also check for the slash-based path in docs and example configs:rg -n 'modules\.common\.multimedia\.gimp|modules/common/multimedia/gimp'modules/system/common/hardware/boot.nix (1)
9-15:Details
❓ Verification inconclusive
Path renamed correctly – remember to migrate
imports& docsThe option tree now lives under
modules.system.hardware.boot. Ensure allimports = [ … ]lists inflake.nix/nixosConfigurationfiles are updated accordingly, otherwise the module will never be instantiated.
🏁 Script executed:
#!/usr/bin/env bash rg -n $'modules\\.hardware\\.boot' --glob '*.nix'Length of output: 48
Verify
importsfor the renamedbootmoduleWe didn’t find any occurrences of
modules.hardware.bootin your.nixfiles, but absence of matches isn’t proof there are no stale paths. Please manually audit allimports = [ … ]blocks in:
flake.nix(and any other flake overlays)- Your
nixosConfigurationfilesEnsure any import of
modules/system/common/hardware/boot.nix(or older path references) is updated to the new location under
modules.system.hardware.bootOtherwise the module will not be instantiated.
modules/hm/common/dev/dotnet.nix (1)
17-17: Namespace switch looks correct.The option tree has been migrated to
modules.hm.dev.dotnet; matches thecfgpath above.modules/system/common/dev/flutter.nix (1)
4-4: Namespace update is sound.The move to
modules.system.dev.flutteris consistent with the new hierarchy; no functional impact.Also applies to: 15-15
modules/hm/common/shell/starship.nix (1)
9-9: Starship module path updated correctly.No other behavioural changes – LGTM.
Also applies to: 12-12
modules/system/common/dev/php.nix (1)
4-9: Namespace change looks good.The PHP dev environment is now under
modules.system.dev.php; nothing else altered.modules/hm/common/multimedia/obs.nix (1)
4-8: Namespace update looks correctThe move to
modules.hm.multimedia.obsis consistent with the wider refactor and does not introduce functional changes.modules/system/common/dev/flatpak.nix (1)
9-13:Details
✅ Verification successful
Double-check for stale references to the old
modules.dev.flatpakpathThe rename is fine, but any forgotten imports/options will now break evaluation.
Run a quick search to catch leftovers:
🏁 Script executed:
#!/bin/bash # List files still referring to the old namespace rg -n $'modules\.dev\.flatpak'Length of output: 31
No stale references to the old
modules.dev.flatpaknamespace
A search formodules.dev.flatpakreturned no matches—there are no remaining imports or options to update.modules/hm/common/utilities/kde-connect.nix (1)
6-10: Namespace change acknowledged – no additional issues spottedThe path update to
modules.hm.utilities.kde-connectis consistent with the refactor; logic remains sound.modules/hm/common/multimedia/parsec.nix (1)
4-8: Path migration looks goodThe Parsec module correctly adopts the new
modules.hm.multimedianamespace; behaviour unchanged.modules/hm/common/extra/ignore-file-retriever.nix (1)
4-5: Namespace update looks correct.The rename to
modules.hmkeeps the module in sync with the refactor. No additional issues spotted.modules/hm/common/communication/teams.nix (1)
4-8: LGTM – namespace bump onlyThe change to
modules.hm.communication.teamsis consistent with the new naming
scheme.modules/hm/common/shell/navi.nix (1)
4-8: Namespace update confirmedThe option path update matches the repo-wide refactor. No functional changes.
modules/system/common/nix/linux-cachyos.nix (1)
10-17: Correct move tomodules.system.nix.*The
cfgpath andoptionsblock were updated consistently. Looks good.modules/hm/common/dev/python.nix (1)
5-6: Path refactor looks good
cfgnow targetsmodules.hm.dev.python; everything else compiles.modules/hm/common/shell/ranger.nix (1)
3-15: Namespace change looks safe – double-check parent importsThe switch to
modules.hm.shell.rangeris consistent with the new hierarchy and the conditional block is still correct.
Please make sure every higher-levelimports = [ … ]list (e.g.modules/hm/common/default.nixor any flake overlay) has been updated to point at the new location, otherwise the option set will be silently ignored at evaluation time.
No other issues spotted.modules/hm/common/dev/global-tools/act-github.nix (1)
3-14: LGTMPath update aligns with the new hierarchy and the conditional package inclusion is intact.
No further action required.modules/hm/common/documentation/obsidian.nix (1)
4-4: Namespace update looks correct
The move tomodules.hm.documentation.obsidianis consistent with the repo-wide rename; nothing else changes functionally.Also applies to: 122-158
modules/hm/common/shell/tools.nix (1)
4-8: LGTM – namespace bump only
The path change tomodules.hm.shell.toolsis straightforward and keeps behaviour intact.modules/system/common/security/password-manager.nix (1)
7-10: Keep backend list in sync with the mappingSame remark as for the CLI module: the enum can be derived automatically.
- type = lib.types.listOf (lib.types.enum (builtins.attrNames passwordManagerToPackage)); + type = lib.types.listOf (lib.types.enum (builtins.attrNames passwordManagerToPackage));(Just highlighting; you already do this here—good job.)
TODO.md (1)
11-11: No code impact – skipping.modules/system/hosts/fern/default.nix (1)
12-30: Confirm OpenRGB removal & new namespace compile
openrgb.niximport was dropped and the subtree moved undermodules.system.
Please run anixos-rebuild dry-runfor hostfernto ensure:
- No residual references to the removed OpenRGB options remain.
- All nested attributes (
modules.system.games,modules.system.fern.*) are defined by the refactored modules and do not clash.Let me know if you need a quick grep script to verify this automatically.
modules/hm/common/dev/node/node.nix (1)
4-4:Details
✅ Verification successful
Verify downstream references after namespace migration
The option path has been migrated to
modules.hm.dev.node.
Please make sure:
- All declarations in
home.nixor higher-level overlays now use the new path.- Any
cfg = config.modules.common.dev.nodebindings that might still exist in sibling modules are cleaned up.You can quickly validate with:
Also applies to: 30-30
🏁 Script executed:
#!/usr/bin/env bash # Find remaining references to the old namespace rg -n $'modules\\.common\\.dev\\.node'Length of output: 36
Confirmed: No remaining references to
modules.common.dev.node
Ranrg -n 'modules\.common\.dev\.node'against the codebase and found no matches. All declarations now usemodules.hm.dev.node, so no further updates are required.modules/system/hosts/fern/drivers.nix (1)
10-10: Cross-check host configuration after moving tomodules.system.*
cfgand the option set were re-rooted undermodules.system.fern.drivers.
Confirm that the host’sconfiguration.nix(or anyimports) has been updated accordingly; otherwise the option will silently be ignored and the driver stack won’t activate.Also applies to: 19-19
modules/system/hosts/fern/vfio/default.nix (1)
10-10: Namespace change looks good – remember to adjust overridesThe VFIO module now lives under
modules.system.fern.vfio.
Double-check anymkOverride,mkAfter, ormkForcestatements in other fern-specific modules that still target the old path.Also applies to: 26-26
modules/hm/common/utilities/scalar.nix (1)
6-6: Consistent with HM namespace – LGTMScalar’s option path has been correctly migrated to
modules.hm.utilities.scalar, and the remaining code already uses the newcfg. No further action required.Also applies to: 65-65
modules/hm/common/dev/node/pm2.nix (1)
4-8: Namespace refactor looks sound.
cfgand the option path were migrated to the newmodules.hmhierarchy consistently with the rest of the tree.
No functional regressions spotted here.modules/hm/common/dev/node/prisma.nix (1)
4-8: Namespace refactor OK.The option path update is consistent with the new layout.
modules/hm/common/dev/editor/android-studio.nix (1)
4-8: Namespace change consistent.No issues with the refactor itself.
modules/system/common/default.nix (1)
18-28: Verify new import paths exist.
./networks/vpn.nixand./virtualisation/containers.nixmust have been moved/created in the same commit; otherwise the system config will fail to evaluate.Please run
nixos-rebuild dry-run(or equivalent CI eval) to confirm the paths resolve.modules/system/common/networks/vpn.nix (1)
20-22: Good, declarative toggle for TailscaleThe conditional service enablement is concise and idiomatic. 👍
modules/system/common/virtualisation/containers.nix (2)
14-18: Nice pluralisation & list-based refactorMoving from a single enum to
enginesas a list greatly improves flexibility. No issues spotted here.
21-33: Verifyvars.useris always defined
users.groups.docker.members = [ "${vars.user}" ];will raise an evaluation error ifvars.useris unset for some hosts.
Please ensure every consuming host definesvars.user, or guard the assignment:(lib.mkIf (vars ? user) { users.groups.docker.members = [ vars.user ]; })modules/hm/common/communication/mail/default.nix (1)
8-12: Mapping now returns lists – good callWrapping each package in a list simplifies later flattening. Implementation looks solid.
modules/hm/common/games/games.nix (1)
8-11: Verify necessity of heavy toolchain packagesIncluding
gccandglibcinhome.packagesfor Minecraft inflates closure size by hundreds of MB. Unless these are strictly required at runtime (e.g., PrismLauncher cannot resolve them from the system), consider dropping them or usingpkgs.stdenv.cc.cc.lib.modules/hm/hosts/oak/default.nix (1)
20-26: Doesgames.mangohudexist in the new games module?
modules.hm.gameswas consolidated into a singlegames.nix, but this host file still referencesgames.mangohud. Unless there is a separatemangohud.nixproviding that option, this path will now be undefined and evaluation will fail.modules/hm/common/browser/default.nix (1)
30-35: Driver gating and deduplication look solid
driversis conditionally included vialib.optionalsand the finalhome.packageslist is deduplicated withlib.unique. Nice!modules/system/hosts/oak/default.nix (2)
15-17: Ensure host config tracks future schema changesThe host now opts into
modules.system.games.clients = [ "steam" "lutris" ]; modules.system.networks.vpn = [ "tailscale" ];Good, but keep an eye on the upstream modules: if they rename
vpn→vpns(plural) or add validation against unknown strings, this host profile will break at evaluation time.
24-29: Antivirus GUI flag aligns with new module – LGTM
gui.enable = true;matches the option introduced in the refactored antivirus module.modules/hm/common/emulator/default.nix (1)
27-33: 🧹 Nitpick (assertive)Package duplication still possible – wrap with
lib.uniqueearlier
baseEmulatorPackagesis later de-duplicated vialib.unique, butadditionalPackages
can already contain Wine or Proton packages included inbaseEmulatorPackages
(e.g. if someone wrongly adds"proton"toemulatorsand also setsproton.enable = true).A simpler, always-unique expression avoids that:
- home.packages = lib.unique (baseEmulatorPackages ++ additionalPackages); + home.packages = lib.unique ( + baseEmulatorPackages + ++ additionalPackages + );Minor, yet keeps deterministic ordering.
Likely an incorrect or invalid review comment.
modules/system/common/games/games.nix (1)
74-84:programs.gamemode.settings.customis not a recognised keyUpstream
programs.gamemode.settingsonly accepts the INI-sections
general,gpu,customcommands must be placed undergeneralas
start/end. Verify with:nixpkgs#programs.gamemodeAdjust or HM will fail to evaluate.
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
modules/system/common/dev/flutter.nix (1)
47-72: Consider adding PATH configuration for Flutter development tools.The current configuration sets environment variables but doesn't appear to modify PATH to include Flutter SDK, Android SDK tools, or Java binaries. This could cause issues where Flutter commands and related tools aren't accessible from the shell.
Consider adding PATH modifications to ensure development tools are accessible:
config = lib.mkIf cfg.enable { environment.systemPackages = [ pkgs.flutter cfg.jdkPackage ] ++ lib.optional cfg.withAndroid androidSdk; # Environment variables environment.variables = lib.mkMerge [ { JAVA_HOME = "${cfg.jdkPackage}"; } #{ STUDIO_JDK = "${cfg.jdkPackage}"; } (lib.mkIf cfg.withAndroid { ANDROID_HOME = "${androidSdk}/libexec/android-sdk"; }) ]; + # Add development tools to PATH + environment.systemPath = lib.mkMerge [ + [ "${pkgs.flutter}/bin" ] + [ "${cfg.jdkPackage}/bin" ] + (lib.mkIf cfg.withAndroid [ + "${androidSdk}/libexec/android-sdk/tools" + "${androidSdk}/libexec/android-sdk/platform-tools" + ]) + ]; programs = lib.mkIf cfg.withAndroid { adb.enable = true; };Alternatively, if PATH modification was intentionally removed due to system-level package management, please document this architectural decision.
♻️ Duplicate comments (2)
modules/hm/desktops/hydenix.nix (1)
124-125: Duplicate keybinding overrides the first commandThis issue was already identified in previous reviews. Hyprland keeps the last
bindfor a given key; thereforeALT+Tabwill only executebringactivetotop, silently droppingcyclenext. Combine the actions or remove the duplicate.modules/hm/common/default.nix (1)
35-37: Game module consolidation is effective, but consider import grouping.The replacement of individual game modules with a consolidated
games.nixapproach reduces code duplication and improves maintainability. However, the game-related imports could be better grouped for readability.As noted in the previous review, consider grouping the three game-related imports together:
- ./games/joystick.nix - ./games/mangohud.nix - ./games/games.nix + ./games/games.nix + ./games/joystick.nix + ./games/mangohud.nix
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
modules/hm/common/communication/discord.nix(1 hunks)modules/hm/common/default.nix(2 hunks)modules/hm/common/dev/node/prisma.nix(2 hunks)modules/hm/common/dev/python.nix(3 hunks)modules/hm/desktops/hydenix.nix(3 hunks)modules/hm/hosts/oak/default.nix(3 hunks)modules/system/common/dev/flutter.nix(3 hunks)modules/system/hosts/oak/default.nix(1 hunks)
🔇 Additional comments (20)
modules/hm/desktops/hydenix.nix (3)
12-13: Good modularization approach.The import of the external configuration module with proper argument passing follows NixOS best practices for separating concerns and improving maintainability.
96-96: Clean configuration injection.Using the imported module's keybindings via string interpolation is an effective way to modularize the configuration while maintaining readability.
127-127: Reasonable autostart configuration.The exec-once command with a delay is a good practice for ensuring the desktop environment is fully loaded before launching applications.
modules/system/common/dev/flutter.nix (2)
4-4: LGTM: Namespace migration aligns with PR objectives.The migration from
modules.dev.fluttertomodules.system.dev.flutteris consistent with the broader refactoring described in the PR objectives to reorganize module hierarchies.Also applies to: 15-15
59-59: Verify impact of commenting out STUDIO_JDK environment variable.The
STUDIO_JDKenvironment variable has been commented out, which could affect Android Studio's JDK detection if it's installed on the system. Android Studio typically uses this variable to determine which JDK to use for projects.Please verify if this change is intentional and won't break Android Studio integration:
#!/bin/bash # Description: Check if Android Studio configuration or other modules set STUDIO_JDK # Expected: Find alternative STUDIO_JDK configuration or confirm it's no longer needed echo "Searching for STUDIO_JDK references in the codebase..." rg -A 3 -B 3 "STUDIO_JDK" echo "Searching for Android Studio related configurations..." rg -A 5 -B 5 -i "android.?studio"modules/hm/common/dev/node/prisma.nix (2)
4-4: LGTM: Namespace refactoring is consistent with PR objectives.The namespace changes from
modules.commontomodules.hmalign with the broader module hierarchy refactoring described in the PR objectives.Also applies to: 7-7
17-23: Verify Prisma environment variables and package dependency consistency.The addition of Prisma engine environment variables is a valuable enhancement that helps Prisma locate its engines when installed via Nix. However, please verify the following:
- Package dependency: The module installs
prismabut referencesprisma-enginesin environment variables. Ensure both packages are available and this approach is correct.- Environment variable correctness: Confirm these are the standard Prisma environment variable names.
- Hardcoded checksum ignore: The
PRISMA_ENGINES_CHECKSUM_IGNORE_MISSING = 1should be verified as appropriate for this use case.#!/bin/bash # Description: Verify Prisma packages and environment variables in nixpkgs # Check if both prisma and prisma-engines packages exist echo "Checking nixpkgs for Prisma packages..." rg -A 5 "pname.*=.*\"prisma\"" rg -A 5 "pname.*=.*\"prisma-engines\"" # Search for standard Prisma environment variable usage echo "Searching for Prisma environment variables in nixpkgs..." rg "PRISMA_.*ENGINE.*BINARY" rg "PRISMA_ENGINES_CHECKSUM"modules/hm/common/default.nix (1)
9-9: New Discord module integration looks good.The addition of the Discord communication module aligns well with the modular structure and follows the established pattern for communication tools.
modules/hm/common/communication/discord.nix (1)
7-15: Discord client mappings look comprehensive.The client-to-package mapping covers a good range of Discord variants from stable to development versions, including alternative clients like
cordlessanddiscordo.modules/system/hosts/oak/default.nix (3)
7-7: Namespace migration executed correctly.The change from
modulestomodules.systemaligns with the PR's objective to establish a consistent hierarchical organization.
15-17: List-based configuration improves flexibility.The migration from boolean flags to list-based configurations (e.g.,
games.clients = ["steam"],networks.vpn = ["tailscale"]) provides better extensibility and follows the data-driven approach established throughout the refactoring.
25-25: Antivirus engines list configuration is consistent.The change to
engines = ["clamav"]follows the same pattern as other module consolidations, allowing for multiple antivirus engines to be configured simultaneously.modules/hm/common/dev/python.nix (2)
5-5: Namespace migration properly implemented.The change from
config.modules.common.dev.pythontoconfig.modules.hm.dev.pythonis consistent with the broader refactoring effort.
29-29: Options namespace updated correctly.The options declaration follows the new namespace structure while preserving all the existing configuration capabilities.
modules/hm/hosts/oak/default.nix (6)
11-11: Home Manager namespace migration properly executed.The change from
config.modules.commontoconfig.modules.hmis consistent with the refactoring objectives and maintains configuration compatibility.
24-24: Games configuration simplification is effective.The migration from
minecraft.enabletoenabledGames = ["minecraft"]provides better scalability and aligns with the consolidated games module approach.
31-31: Browser client configuration follows established pattern.The rename from
emulatorstoclientsand the list-based approach is consistent with other module refactoring in the PR.
57-57: Python configuration appropriately simplified.The removal of nested
devShell.enableconfiguration aligns with the simplification of the Python module, removing the shell environment generation functionality.
62-62: Discord RPC integration properly configured.The addition of
discord.rpc.enable = truecorrectly utilizes the new Discord module's RPC functionality.
65-67: Network tunnel configuration follows data-driven approach.The migration to
services = ["localtunnel"]with service-specific configuration maintains functionality while providing better extensibility for multiple tunnel services.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (15)
flake.nix (1)
147-147: Update usage message to include pine option.The usage message still shows the old options and doesn't include pine, as noted in the previous review.
- *) echo "Usage: rb [oak|fern|all]" >&2; exit 1 ;; + *) echo "Usage: rb [oak|fern|pine|all]" >&2; exit 1 ;;modules/hm/common/documentation/default.nix (1)
12-15: Uselib.uniqueto avoid duplicate packagesThe past review comment about using
lib.uniqueto prevent duplicate packages is still applicable. Multiple editors could potentially share the same package dependencies.modules/hm/common/default.nix (1)
35-37: Consider grouping game imports for consistencyThe past review comment about grouping the game-related imports together is still applicable for improved readability.
modules/system/hosts/fern/sunshine.nix (1)
8-12: Consider backward compatibility and IPv6 supportThe past review comments about adding an alias for backward compatibility and including IPv6 firewall rules alongside the existing IPv4 rules are still relevant considerations.
modules/hm/common/dev/rust.nix (2)
8-10: Critical type mismatch remains unresolvedThe error handling with
abortsuggests awareness of the underlying issue, but the fundamental problem persists:extraPackagesis declared aslistOf packagebut used aslistOf strfor package name lookups.Fix the type declaration to match usage:
extraPackages = lib.mkOption { - type = lib.types.listOf lib.types.package; + type = lib.types.listOf lib.types.str;
8-10: Performance: Wrap expensive computation in conditionalThe
rustPackagescomputation runs unconditionally, causing unnecessary work when the module is disabled.Move the computation inside the config conditional:
-rustPackages = rustDefaultPackages - ++ (map (pkgName: pkgs.${pkgName} or (abort "Unknown Rust pkg: ${pkgName}")) - cfg.extraPackages); in { # options... config = lib.mkIf cfg.enable { + home.packages = rustDefaultPackages + ++ (map (pkgName: pkgs.${pkgName} or (abort "Unknown Rust pkg: ${pkgName}")) + cfg.extraPackages); - home.packages = rustPackages; };modules/hm/common/communication/mail/default.nix (1)
14-15: Unresolved: Add deduplication for package safetyThe previous review comment about potential duplicates from
lib.concatMapremains unaddressed. While unlikely with current services, addinglib.uniquewould prevent future issues.-packagesToInstall = lib.concatMap (service: serviceToPackage.${service}) serviceList; +packagesToInstall = lib.unique (lib.concatMap (service: serviceToPackage.${service}) serviceList);modules/hm/common/engine/default.nix (1)
4-4: Unresolved: cfg scope is too broadThe previous review comment about
cfg = config.modules.hmbeing too broad remains unaddressed, creating potential naming conflicts.-cfg = config.modules.hm; +cfg = config.modules.hm.engine;modules/hm/common/dev/global-tools/cli.nix (1)
27-27: Addresses previous review: Deduplication withlib.uniqueThe addition of
lib.uniqueproperly addresses the previous review comment about preventing duplicate packages inhome.packages.modules/system/common/security/antivirus.nix (2)
6-10: Early evaluation issue persistsThe
engineToPackagesmapping still referencescfg.gui.enablebefore the options are defined, which can cause evaluation loops or "access to undefined option" errors.Consider refactoring to avoid this recursion risk:
- engineToPackages = { - clamav = [ pkgs.clamav ] ++ lib.optionals cfg.gui.enable [ pkgs.clamtk ]; - sophos = [ pkgs.sophos-av ]; - }; + staticEnginePkgs = { + clamav = [ pkgs.clamav ]; + sophos = [ pkgs.sophos-av ]; + }; + + engineToPackages = lib.mapAttrs + (_: pkgs: pkgs ++ lib.optionals cfg.gui.enable [ pkgs.clamtk ]) + staticEnginePkgs;
12-12: UX improvement: "none" handlingThe filtering of
"none"from enabled engines is better than before, but since the default is now an empty list, the "none" option becomes redundant. Consider removing it entirely from the enum for clearer UX.modules/hm/common/network/tunnel.nix (2)
28-35: Applylib.uniqueto prevent duplicate packages.The aggregated
servicesPackagesmay contain duplicates if a service appears multiple times incfg.services, which could cause Home-Manager issues.Apply this diff to prevent duplicate packages:
- home.packages = servicesPackages; + home.packages = lib.unique servicesPackages; home.shellAliases = enabledAliases;
36-45: Directory creation is still missing for generated files.The file configurations assume parent directories exist, but Home-Manager doesn't create intermediate directories automatically.
Add directory creation to ensure parent directories exist:
# Cloudflare token configuration cloudflareTokenConfig = lib.mkIf (builtins.elem "cloudflare" cfg.services && cfg.cloudflare.tokenPath != null) { home.sessionVariables.CLOUDFLARE_TOKEN_FILE = cfg.cloudflare.tokenPath; - home.file.".cloudflared/token".source = cfg.cloudflare.tokenPath; + home.file.".cloudflared/token" = { + source = cfg.cloudflare.tokenPath; + recursive = true; + }; }; # Ngrok config file ngrokConfig = lib.mkIf (builtins.elem "ngrok" cfg.services && cfg.ngrok.configPath != null) { - home.file.".config/ngrok/ngrok.yml".source = cfg.ngrok.configPath; + home.file.".config/ngrok/ngrok.yml" = { + source = cfg.ngrok.configPath; + recursive = true; + }; };modules/system/common/games/games.nix (2)
29-33: Steam compat tools path still uses problematicbuiltins.getEnv "HOME".During evaluation,
HOMEis usually unset, producing an invalid path like/compatibilitytools.d.The default path needs to be derived differently since
builtins.getEnv "HOME"fails during evaluation. Consider removing the default or using a proper user home derivation:steam.compatToolsPath = lib.mkOption { type = lib.types.path; - default = "${builtins.getEnv "HOME"}/.steam/root/compatibilitytools.d"; + # Remove default or derive from user config description = "Path for Steam compatibility tools"; };
70-72: Environment variable merging issue persists.
STEAM_EXTRA_COMPAT_TOOLS_PATHSshould accumulate paths from multiple modules, not overwrite existing values.Use proper merging for the environment variable:
environment.sessionVariables = lib.mkIf (lib.elem "steam" cfg.clients) { - STEAM_EXTRA_COMPAT_TOOLS_PATHS = cfg.steam.compatToolsPath; + STEAM_EXTRA_COMPAT_TOOLS_PATHS = lib.mkMerge [ cfg.steam.compatToolsPath ]; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
flake.lockis excluded by!**/*.lock
📒 Files selected for processing (19)
flake.nix(4 hunks)modules/hm/common/browser/default.nix(2 hunks)modules/hm/common/communication/mail/default.nix(1 hunks)modules/hm/common/default.nix(2 hunks)modules/hm/common/dev/global-tools/cli.nix(1 hunks)modules/hm/common/dev/rust.nix(1 hunks)modules/hm/common/documentation/default.nix(1 hunks)modules/hm/common/emulator/default.nix(3 hunks)modules/hm/common/engine/default.nix(1 hunks)modules/hm/common/extra/ignore-file-retriever.nix(3 hunks)modules/hm/common/network/tunnel.nix(3 hunks)modules/system/common/default.nix(2 hunks)modules/system/common/games/games.nix(3 hunks)modules/system/common/networks/wol.nix(1 hunks)modules/system/common/security/antivirus.nix(1 hunks)modules/system/hosts/fern/default.nix(1 hunks)modules/system/hosts/fern/drivers.nix(0 hunks)modules/system/hosts/fern/sunshine.nix(1 hunks)modules/system/hosts/oak/default.nix(1 hunks)
💤 Files with no reviewable changes (1)
- modules/system/hosts/fern/drivers.nix
🧰 Additional context used
🪛 GitHub Actions: Flake Check
flake.nix
[error] 105-106: Nix flake check failed: The option modules.system.games.steam.enable does not exist. Error triggered during evaluation of nixosConfigurations in flake.nix at line 105.
🔇 Additional comments (41)
flake.nix (8)
28-31: LGTM: nix-darwin input properly configured.The nix-darwin input follows the same pattern as other inputs and correctly follows nixpkgs to maintain version consistency.
34-34: LGTM: Outputs function properly updated for nix-darwin.The function signature correctly includes the nix-darwin parameter to use the newly added input.
54-66: LGTM: Well-structured macOS configuration function.The
mkDarwinHostfunction follows the same pattern asmkHostand properly handles macOS system configuration. The hardcoded system architecture is acceptable with the explanatory comment.
114-117: LGTM: Darwin configurations properly structured.The darwinConfigurations section correctly uses the new mkDarwinHost function, and the comment provides helpful guidance for customization.
122-122: LGTM: Pine deployment node properly configured.The pine deployment node follows the established pattern and correctly targets the local variant.
140-141: LGTM: Pine deployment logic properly implemented.The pine deployment cases are correctly added to both individual and batch deployment logic.
Also applies to: 145-145
159-161: LGTM: Pine deployment checks properly configured.The pine deployment checks follow the established pattern and are correctly integrated with the deploy-rs framework.
108-108: We need to see ifhosts/pine/default.nixactually references anymodules.*paths:#!/bin/bash grep -R -n "modules\." hosts/pine/default.nix || echo "No module references in pine host config"modules/hm/common/documentation/default.nix (1)
4-4: Namespace change looks correctThe refactoring from
modules.common.documentationtomodules.hm.documentationaligns with the broader namespace reorganization across the codebase.Also applies to: 18-18
modules/system/common/networks/wol.nix (1)
8-8: Namespace reorganization looks correctThe change from
modules.fern.woltomodules.system.networks.wolproperly groups the Wake-on-LAN module under the system networks hierarchy, consistent with the broader module reorganization.Also applies to: 11-11
modules/hm/common/default.nix (1)
9-9: Module consolidation and additions look goodThe addition of Discord communication module, enabling of ignore-file-retriever, and consolidation of individual game modules into a unified games.nix module improves maintainability and organization.
Also applies to: 33-33, 37-37
modules/system/common/default.nix (1)
19-20: Import path updates align with module consolidationThe simplification of import paths (
vpn/tailscale.nix→vpn.nix,containers/containers.nix→containers.nix) and addition ofwol.nixproperly reflect the module consolidation and namespace reorganization.Also applies to: 29-29
modules/system/hosts/fern/sunshine.nix (1)
8-8: Namespace change is correctThe refactoring from
modules.fern.sunshinetomodules.system.fern.sunshineproperly aligns with the system namespace reorganization.Also applies to: 11-11
modules/system/hosts/fern/default.nix (1)
10-30: LGTM: Clean namespace migration improves module organizationThe migration from
modulestomodules.systemnamespace is well-executed. The restructuring with dedicatednetworksandfernsections provides better logical grouping of configuration options.modules/hm/common/browser/default.nix (2)
6-34: Excellent mapping-based refactoringThe introduction of
browserToPackageandbrowserToDrivermappings with dynamic package collection is a clean, maintainable approach. The null filtering and deduplication logic is well-implemented.
39-42: Good: Dynamic enum generation ensures consistencyUsing
lib.attrNames browserToDriverfor the enum type ensures that available options stay in sync with the mapping definition, preventing configuration drift.modules/hm/common/engine/default.nix (1)
15-21: Inconsistent option path creates scope mismatchOptions are declared under
modules.hmbut accessed ascfg.engine, creating a mismatch. This should bemodules.hm.engineto align with the intended scope.-options.modules.hm = { +options.modules.hm.engine = { engine = lib.mkOption {Then update the cfg binding accordingly:
-cfg = config.modules.hm; +cfg = config.modules.hm.engine;Likely an incorrect or invalid review comment.
modules/hm/common/extra/ignore-file-retriever.nix (4)
4-4: LGTM: Namespace migration tomodules.hmThe configuration namespace has been correctly updated to align with the broader migration from
modules.commontomodules.hmseen across the codebase.
6-11: Good improvement: Path normalizationThe path expansion function now properly normalizes paths by removing trailing slashes before processing, which enhances consistency in path handling.
69-69: Correct use of double single-quote escapingThe string interpolation has been properly updated to use Nix's
''${variable}''syntax for shell script generation, which correctly handles escaping within the embedded shell script.Also applies to: 74-74, 77-77
91-91: Consistent options namespace updateThe options declaration correctly reflects the new
modules.hm.extra.ignore-file-retrievernamespace, maintaining consistency with the configuration reference.modules/hm/common/dev/global-tools/cli.nix (3)
4-4: LGTM: Namespace migration tomodules.hmThe configuration namespace has been correctly updated to align with the broader migration pattern across the codebase.
6-14: Excellent refactoring: Clean mapping-based approachThe introduction of
cliToPackagemapping and the streamlined package selection logic usinglib.filterandmapsignificantly improves maintainability and readability compared to the previous conditional approach.
18-23: Good practice: Dynamic enum generationUsing
lib.attrNames cliToPackagefor the enum ensures the option definition stays automatically synchronized with the package mapping, preventing drift between allowed values and available packages.modules/system/hosts/oak/default.nix (3)
7-7: LGTM: Consistent namespace migrationThe update to
modules.systemnamespace aligns with the systematic reorganization of system modules across the codebase.
15-17: Good modernization: List-based configurationThe migration from individual boolean flags to list-based configuration (
games.clients,networks.vpn,virtualisation.containers.engines) provides better flexibility by supporting multiple simultaneous selections rather than single choices.
25-25: Consistent pattern: Multiple antivirus enginesThe change from a single
enginetoengineslist follows the same pattern as other modules, allowing multiple antivirus engines to be enabled simultaneously.modules/system/common/security/antivirus.nix (3)
4-4: LGTM: Namespace migration tomodules.systemThe configuration namespace has been correctly updated to align with the system module organization.
17-21: Good improvement: List-based engines configurationThe migration from single engine enum to a list of engines provides better flexibility and aligns with the pattern used across other modules in this refactoring.
24-32: Clean package aggregation and service configurationThe use of
lib.concatMapfor package aggregation and conditional service enabling based on engine presence is well-structured and maintainable.modules/hm/common/emulator/default.nix (5)
4-4: LGTM: Namespace migration tomodules.hmThe configuration namespace has been correctly updated to align with the broader migration pattern across Home Manager modules.
6-32: Excellent modularization: Structured package mappingsThe introduction of
emulatorToPackage,winePackages, andprotonPackagesmappings significantly improves code organization and maintainability. The streamlined package aggregation logic is much cleaner than the previous approach.
38-38: Breaking change: Default emulators listThe default has been changed from
["wine"]to an empty list. This is a breaking change that users should be aware of, though it provides more explicit control over enabled emulators.
51-52: Simplified wine versions: "staging" removedThe wine version enum has been simplified by removing the "staging" option and changing the default to "stable". Ensure this doesn't break existing configurations that might have been using the staging version.
74-74: Good practice: Package deduplicationThe use of
lib.uniqueprevents duplicate packages inhome.packages, which is a good defensive practice when aggregating packages from multiple sources.modules/hm/common/network/tunnel.nix (3)
4-11: LGTM! Clean namespace migration and service mapping.The refactor from
modules.common.network.tunneltomodules.hm.network.tunnelaligns with the broader namespace reorganization. TheservicesToPackagemapping provides a clean, data-driven approach for package selection.
49-53: Well-designed dynamic enum type definition.The use of
lib.attrNames servicesToPackagefor the enum type ensures the option stays in sync with available services automatically. This is a best practice for maintainability.
90-99: Clean configuration merge pattern.The use of
lib.mkMergewith separate conditional configurations provides a clean separation of concerns and makes the module logic easy to follow.modules/system/common/games/games.nix (3)
6-16: Excellent refactor to data-driven client selection.The
clientToPackagemapping and dynamic package aggregation significantly improves maintainability compared to individual boolean flags. The pattern is consistent with other modules in this PR.
87-102: Well-structured conditional Steam configuration.The conditional enabling of Steam-related programs based on client list membership is clean and follows NixOS best practices. The separation of gamescope and steam program configurations is logical.
69-73: Environment variable should uselib.uniquefor packages.Following the same pattern as the tunnel module,
clientPackagesshould be deduplicated to prevent issues with duplicate package paths.Apply this diff:
- environment.systemPackages = lib.unique clientPackages; + environment.systemPackages = lib.unique clientPackages;Wait, the code already has
lib.unique. The Steam environment variable configuration looks correct.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (6)
flake.nix (1)
147-147: LGTM: Usage message correctly updated.The usage message now includes all valid options including the new pine host.
hosts/pine/default.nix (1)
78-78: Remove commented initialPassword line for security.The commented initial password line should be removed entirely to prevent accidental uncommenting in production.
- #initialPassword = "${vars.user}"; + # WARNING: Never set initialPassword in production configurationsmodules/hm/common/dev/editor/jetbrains.nix (1)
4-7: Add backward compatibility alias for the old namespace.The namespace migration from
modules.common.dev.editor.jetbrainstomodules.hm.dev.editor.jetbrainslacks a compatibility alias, which could break existing user configurations.modules/hm/common/games/games.nix (1)
20-21: Fix invalid attribute names with hyphens.The bare identifiers
rocket-leagueandstar-citizencontain hyphens, which are invalid in Nix and will cause evaluation failures. These must be quoted as strings.Also applies to: 24-25
modules/system/common/games/games.nix (2)
31-31: Default Steam compat-tools path uses unreliable environment variable.The use of
builtins.getEnv "HOME"during evaluation is problematic sinceHOMEis typically unset, resulting in an invalid path like/compatibilitytools.d.Consider deriving from the target user's home directory or removing the default:
- default = "${builtins.getEnv "HOME"}/.steam/root/compatibilitytools.d"; + # Remove default and require explicit configuration + # default = "${config.users.users.${config.mainUser}.home}/.steam/root/compatibilitytools.d";
71-71: Environment variable overwrites instead of accumulating paths.
STEAM_EXTRA_COMPAT_TOOLS_PATHSshould accumulate paths from multiple modules rather than overwriting existing values.- STEAM_EXTRA_COMPAT_TOOLS_PATHS = cfg.steam.compatToolsPath; + STEAM_EXTRA_COMPAT_TOOLS_PATHS = lib.mkMerge [ + (lib.mkDefault "") + cfg.steam.compatToolsPath + ];Or use colon-separated concatenation if multiple paths need to be supported.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (14)
flake.nix(4 hunks)hosts/oak/default.nix(1 hunks)hosts/pine/default.nix(1 hunks)modules/hm/common/communication/discord.nix(1 hunks)modules/hm/common/communication/mail/default.nix(1 hunks)modules/hm/common/default.nix(2 hunks)modules/hm/common/dev/editor/jetbrains.nix(2 hunks)modules/hm/common/documentation/default.nix(1 hunks)modules/hm/common/games/games.nix(1 hunks)modules/hm/common/network/tunnel.nix(3 hunks)modules/system/common/games/games.nix(3 hunks)modules/system/common/hardware/powersave.nix(2 hunks)modules/system/common/security/antivirus.nix(1 hunks)modules/system/common/security/password-manager.nix(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Flake Check
flake.nix
[error] 105-106: Nix flake check failed due to invalid option 'modules.system.games.steam.enable' which does not exist. Error triggered in nixosConfigurations.fern configuration.
🔇 Additional comments (36)
flake.nix (5)
28-31: LGTM: Clean addition of nix-darwin input.The nix-darwin input is properly configured with nixpkgs following to maintain consistency.
54-66: LGTM: Well-structured macOS host configuration function.The
mkDarwinHostfunction follows the same pattern asmkHostand correctly usesdarwinSystemfor macOS configurations. The system architecture and module path are appropriate.
108-108: LGTM: Consistent integration of pine host.The pine host is properly integrated following the established patterns for fern and oak hosts, including both local and non-local variants plus deployment configuration.
Also applies to: 111-111, 122-122
114-117: Consider adding documentation for macOS hostname discovery.The comment provides helpful guidance about finding the actual hostname. This is good practice for macOS configurations.
105-106: Investigate pipeline failure related to module structure changes.The pipeline is failing due to an invalid option
modules.system.games.steam.enablein the fern configuration. This suggests the module namespace reorganization frommodules.commontomodules.systemmentioned in the PR objectives may have introduced inconsistencies.Verify that all module references have been consistently updated across the codebase:
#!/bin/bash # Search for old module namespace references that may not have been updated echo "Searching for old module namespace references..." rg -n "modules\.common" --type nix echo "Searching for steam configuration references..." rg -n "games\.steam" --type nix echo "Checking fern host configuration specifically..." cat hosts/fern/default.nix 2>/dev/null || echo "hosts/fern/default.nix not found"hosts/pine/default.nix (5)
7-21: LGTM: Well-structured package configuration with proper overlays.The nixpkgs configuration properly uses hydenix inputs with overlays and creates a userPkgs overlay for additional packages. The allowUnfree settings are consistently applied.
42-43: LGTM: Appropriate hardware module selection.Using
common-pcandcommon-pc-ssdmodules from nixos-hardware is a good choice for general PC hardware support.
46-46: Good hardware-specific configuration.The HDMI kernel parameter is appropriate for ensuring proper video output on this specific hardware.
69-74: LGTM: Proper hydenix configuration for French locale.The configuration correctly sets up the hydenix desktop environment with French locale and Paris timezone.
79-86: LGTM: Standard user group configuration.The user groups include all necessary permissions for a desktop user (wheel, networkmanager, video, lp, scanner).
modules/hm/common/documentation/default.nix (1)
1-29: LGTM! Clean refactoring with improved maintainability.The namespace migration and dynamic mapping approach are well-implemented. Past review concerns about duplicate packages and safe attribute access have been properly addressed with
lib.uniqueand theor nullpattern.modules/system/common/hardware/powersave.nix (1)
107-123: Battery threshold settings now properly conditional.The charge threshold configuration correctly uses
lib.optionalString cfg.batteryHealth.enableto conditionally apply settings, addressing the previous concern about unconditional application. The dual battery support (BAT0/BAT1) and threshold persistence settings are well-implemented.modules/system/common/security/password-manager.nix (1)
1-58: Well-structured refactoring with improved modularity.The namespace migration and dynamic mapping implementation are excellent. The use of
lib.uniqueproperly addresses the previous concern about package duplicates between password manager and YubiKey packages. The modular config structure withlib.mkMergeenhances readability.modules/hm/common/network/tunnel.nix (1)
98-98: Good use of lib.unique for package deduplication.The implementation properly addresses the previous concern about duplicate packages by using
lib.unique.modules/hm/common/default.nix (3)
9-9: Good addition of Discord communication module.Adding Discord support enhances the communication tools available in the configuration.
33-33: Enabling ignore-file-retriever functionality.Uncommenting this import enables useful file retrieval functionality that was previously disabled.
35-37: Excellent games module consolidation.Replacing individual game imports with a unified
games.niximproves maintainability and reduces import complexity. The addition ofjoystick.nixprovides complementary gaming hardware support.hosts/oak/default.nix (1)
18-21: Good structural consistency improvement!The configuration is now consistent with the outer
pkgsimport structure, grouping options under aconfigattribute. This improves maintainability and readability.modules/hm/common/dev/editor/jetbrains.nix (1)
18-24: Good deduplication implementation!The use of
lib.uniqueproperly handles duplicate IDE entries in the configuration list, preventing redundant package installations.modules/hm/common/games/games.nix (2)
34-34: Excellent deduplication implementation!The use of
lib.uniqueprevents duplicate packages when the same game appears multiple times incfg.enabledGames.
1-48: Well-structured consolidation of game modules.This unified approach to game configuration is much cleaner than having separate modules for each game. The data-driven mapping with dynamic enumerations improves maintainability.
modules/hm/common/communication/mail/default.nix (3)
4-4: Consistent namespace migration!The namespace change from
modules.common.communication.mailtomodules.hm.communication.mailaligns with the broader refactoring across the repository.Also applies to: 18-18
9-15: Excellent dynamic mapping and deduplication implementation!The service-to-package mapping with list support and
lib.uniquededuplication properly handles shared packages between services, preventing duplicates in the final package list.
20-20: Good use of dynamic enumeration!Using
lib.attrNames serviceToPackagefor the enum type ensures the option validation stays in sync with available services automatically.modules/hm/common/communication/discord.nix (3)
49-49: Good default value choice for RPC!Setting
rpc.enable = falseby default prevents automatic inclusion of discord-rpc when users don't explicitly need it.
54-54: Correct conditional logic implementation!The condition properly includes all three cases: clients, overlays, and RPC enable, ensuring packages are installed when any feature is requested.
6-31: Well-structured package mappings!The separation of clients, overlays, and RPC into distinct mappings with proper list handling creates a clean and maintainable configuration structure.
modules/system/common/security/antivirus.nix (5)
4-4: LGTM: Namespace migration aligns with PR objectives.The change from
modules.security.antivirustomodules.system.security.antivirusfollows the systematic namespace reorganization described in the PR objectives.
6-10: LGTM: Well-structured engine-to-package mapping.The
engineToPackagesmapping cleanly associates engine names with their packages, and the conditional GUI package inclusion for ClamAV is properly implemented. This addresses the previous early evaluation concerns sincecfg.gui.enableis accessed after options are defined.
12-14: LGTM: Efficient package aggregation.The use of
lib.concatMapto flatten and collect packages from selected engines is clean and efficient.
19-23: LGTM: List-based approach improves flexibility.The change from a single
engineenum to anengineslist allows multiple antivirus engines to be enabled simultaneously. The removal of "none" from the enum and defaulting to an empty list addresses the previous UX concerns about silent filtering.
29-29: LGTM: Correct service condition logic.Using
lib.elem "clamav" enabledEnginesproperly checks if ClamAV packages are included, ensuring the service is only enabled when the engine is actually selected.modules/system/common/games/games.nix (4)
4-4: LGTM: Namespace migration aligns with PR objectives.The change from
modules.gamestomodules.system.gamesfollows the systematic namespace reorganization described in the PR objectives.
6-12: LGTM: Clean client-to-package mapping.The
clientToPackagemapping provides a clear association between client names and their packages, supporting the new list-based approach effectively.
19-26: LGTM: Unified client selection improves maintainability.The change from individual boolean enable flags to a unified
clientslist option enhances flexibility and follows the consistent pattern described in the PR objectives. The description correctly includes all supported clients including "nexus".
87-101: LGTM: Conditional program configuration based on client selection.The Steam-specific program configuration is properly conditional on "steam" being in the clients list, maintaining clean separation of concerns.
|
🎉 This PR is included in version 1.2.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
This pull request introduces significant refactoring and enhancements to the configuration modules, focusing on improving maintainability, consistency, and modularity. The changes include renaming configuration paths, introducing reusable mappings for packages, and simplifying package selection logic. Below is a summary of the most important changes grouped by theme.
Refactoring of Configuration Paths
config.modules.commontoconfig.modules.hmacross multiple modules to align with the new naming convention. This affects modules for browsers, communication tools, development environments, and documentation. [1] [2] [3] [4] [5]Modularization of Package Mappings
Simplification of Package Selection Logic
lib.concatMapandlib.filterfunctions. This change ensures a cleaner and more readable configuration. [1] [2] [3] [4]Removal of Redundant Configurations
emulatorsoption in the browser module, consolidating functionality into the new mappings and package selection logic.Miscellaneous Enhancements
games.nixfile for better organization.TODO.mdfile to reflect completed tasks and remove unnecessary items. [1] [2]## DescriptionType of change
Checklist