refactor: darwin modules and add explicit flake checks - #32
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds Starship prompt config, updates Zed settings, refactors Darwin host/module configs (shared imports, new modules for determinate Nix and nerd fonts), changes wallpaper option/activation, restructures flake.nix for multi-system checks/formatters, and enables runtime switching between Powerlevel10k and Starship. Changes
Sequence DiagramsequenceDiagram
participant User
participant Shell as Zsh Shell
participant ThemeCheck as PROMPT_THEME Check
participant P10k as Powerlevel10k Init
participant Starship as Starship Init
participant P10kCfg as p10k.zsh
User->>Shell: start shell
Shell->>ThemeCheck: evaluate PROMPT_THEME
alt PROMPT_THEME == "p10k"
ThemeCheck->>P10k: load Powerlevel10k
P10k->>P10kCfg: source p10k config (guarded)
P10kCfg->>User: render p10k prompt
else
ThemeCheck->>Starship: initialize Starship
Starship->>User: render Starship prompt
end
User->>Shell: run prompt-starship or prompt-p10k (runtime switch)
Shell->>ThemeCheck: re-evaluate PROMPT_THEME and re-init chosen prompt
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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. Comment |
PR Review: refactor: darwin modules and add explicit flake checksOverall this is a solid refactor — the changes move in the right direction. The DRY improvements, better option shapes, and explicit CI checks are all worthwhile. A few issues worth addressing: Bugs / Issues1. In # darwin/modules/wallpaper.nix
config = lib.mkIf cfg.enable {
homebrew.casks = [ "desktoppr" ];
...
};2. Hardcoded path in out-of-store Zed symlink config.lib.file.mkOutOfStoreSymlink "${config.home.homeDirectory}/src/dotfiles/.config/zed/settings.json"This silently creates a dangling symlink if the repo isn't checked out at 3. The PR description says "remove Minor Issues4. Non-atomic write to In cat "$tmp" > /etc/nix/nix.custom.conf
rm -f "$tmp"If the system loses power between these two lines the file could be left empty. Since mv "$tmp" /etc/nix/nix.custom.conf5.
Positive notes
🤖 Generated with Claude Code |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@darwin/hosts/dunston.nix`:
- Line 1: The module currently uses a catch-all signature "{ ... }:" which hides
expected inputs; update the function signature in dunston.nix to explicitly
destructure the standard host module attributes (e.g. use "{ pkgs, lib, config,
... }:"), ensuring pkgs/lib/config are available for use in the file and
preserving the rest via "...".
In `@darwin/hosts/mfourmax.nix`:
- Line 1: The file currently uses a generic anonymous argument signature "{ ...
}:"; update it to the repo's destructured attribute-set form by replacing that
with an explicit destructured signature such as "{ pkgs, lib, config, ... }:" so
callers and internal references clearly see the provided attributes; locate the
top-level function signature in mfourmax.nix and adjust it to the explicit
destructured form (e.g., include pkgs, lib, config) to match the repository
convention.
In `@darwin/hosts/shared/media.nix`:
- Line 1: Replace the generic module signature "{ ... }:" with an explicit
destructured attribute set (for example "{ pkgs, lib, config, ... }:"), updating
the function parameter list in this module's top-level signature so it follows
the repo convention and makes required inputs (pkgs, lib, config) explicit.
In `@darwin/hosts/spaceblack.nix`:
- Line 1: The module currently accepts a catch-all argument set "{ ... }:";
change it to explicit module arg destructuring like "{ pkgs, lib, config, ...
}:" so the host module exposes standard attributes (pkgs, lib, config) per repo
convention; update the function signature in the top-level of spaceblack.nix to
replace "{ ... }:" with the explicit destructured form and keep the trailing
"...": to preserve additional args.
In `@darwin/modules/determinate-nix.nix`:
- Around line 26-27: Update the line that filters out existing trusted-users
entries so it also ignores lines with leading whitespace: change the grep -v
invocation that currently matches '^trusted-users' to a pattern that allows
optional leading whitespace (for example using a POSIX whitespace class or
equivalent) so indented entries are removed; keep the subsequent echo that
writes "trusted-users = root ${trustedUsers}" to "$tmp" unchanged. This targets
the grep -v ... /etc/nix/nix.custom.conf > "$tmp" || true command and the echo
"trusted-users = root ${trustedUsers}" >> "$tmp" action.
In `@darwin/modules/wallpaper.nix`:
- Around line 30-31: The current invocation uses a hardcoded path
(/usr/local/bin/desktoppr) in the line "sudo -u ${config.system.primaryUser}
/usr/local/bin/desktoppr \"${cfg.image}\"", which breaks on aarch64-darwin;
replace it with a runtime-resolved binary path or the package-provided path. Fix
by changing the call to either use the package reference
${pkgs.desktoppr}/bin/desktoppr (preferred if you add desktoppr to pkgs) or
resolve at runtime with something like invoking sh -c "cmd=$(command -v
desktoppr) && \"$cmd\" \"${cfg.image}\"" under sudo -u
${config.system.primaryUser}; ensure the replacement still runs as the
primaryUser and passes "${cfg.image}" unchanged.
In `@home-manager/default.nix`:
- Around line 18-19: The zed settings symlink hard-codes the dotfiles path;
update xdg.configFile."zed/settings.json".source to use a repo-root variable
instead of "${config.home.homeDirectory}/src/dotfiles/...": accept the dotfiles
repo root as a module option (e.g. an attribute like dotfilesRepoRoot) or via
specialArgs, and pass that variable into the expression used by
config.lib.file.mkOutOfStoreSymlink so the symlink points to
"${dotfilesRepoRoot}/.config/zed/settings.json" rather than a fixed
~/src/dotfiles path.
In `@home-manager/p10k/p10k.zsh`:
- Line 2: The theme-skip guard currently uses a bare return which can propagate
a non-zero status; update the guard in p10k.zsh (the conditional using [[
${PROMPT_THEME:-p10k} == p10k ]] || return) to explicitly return success by
changing the bare return to return 0 so sourcing the file is a graceful no-op
when PROMPT_THEME != p10k.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2c56b1dd-f54d-4430-b8fc-ac8af3f3703a
📒 Files selected for processing (22)
.config/starship.toml.config/zed/settings.jsonAGENTS.mdREADME.mddarwin/darwin.nixdarwin/hosts/benson.nixdarwin/hosts/dunston.nixdarwin/hosts/higgins.nixdarwin/hosts/mfourmax.nixdarwin/hosts/shared/gaming.nixdarwin/hosts/shared/media.nixdarwin/hosts/spaceblack.nixdarwin/modules/activation.nixdarwin/modules/base.nixdarwin/modules/defaults.nixdarwin/modules/determinate-nix.nixdarwin/modules/homebrew.nixdarwin/modules/nerd-fonts.nixdarwin/modules/wallpaper.nixflake.nixhome-manager/default.nixhome-manager/p10k/p10k.zsh
💤 Files with no reviewable changes (1)
- darwin/modules/activation.nix
| @@ -1,12 +1,13 @@ | |||
| { pkgs, lib, ... }: | |||
| { ... }: | |||
There was a problem hiding this comment.
Keep host module signatures explicit.
Line 1 uses { ... }:; please use explicit destructuring per repo convention.
Suggested change
-{ ... }:
+{ pkgs, lib, config, ... }:As per coding guidelines: Use attribute sets with destructuring in function signatures: { pkgs, lib, config, ... }:.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| { ... }: | |
| { pkgs, lib, config, ... }: |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@darwin/hosts/dunston.nix` at line 1, The module currently uses a catch-all
signature "{ ... }:" which hides expected inputs; update the function signature
in dunston.nix to explicitly destructure the standard host module attributes
(e.g. use "{ pkgs, lib, config, ... }:"), ensuring pkgs/lib/config are available
for use in the file and preserving the rest via "...".
| @@ -1,9 +1,10 @@ | |||
| { pkgs, lib, ... }: | |||
| { ... }: | |||
There was a problem hiding this comment.
Use the standard explicit signature form here too.
Line 1 should follow the repo’s destructured function signature convention.
Suggested change
-{ ... }:
+{ pkgs, lib, config, ... }:As per coding guidelines: Use attribute sets with destructuring in function signatures: { pkgs, lib, config, ... }:.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| { ... }: | |
| { pkgs, lib, config, ... }: |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@darwin/hosts/mfourmax.nix` at line 1, The file currently uses a generic
anonymous argument signature "{ ... }:"; update it to the repo's destructured
attribute-set form by replacing that with an explicit destructured signature
such as "{ pkgs, lib, config, ... }:" so callers and internal references clearly
see the provided attributes; locate the top-level function signature in
mfourmax.nix and adjust it to the explicit destructured form (e.g., include
pkgs, lib, config) to match the repository convention.
| @@ -0,0 +1,9 @@ | |||
| { ... }: | |||
There was a problem hiding this comment.
Align module signature with repo convention.
Line 1 should use explicit destructuring instead of { ... }:.
Suggested change
-{ ... }:
+{ pkgs, lib, config, ... }:As per coding guidelines: Use attribute sets with destructuring in function signatures: { pkgs, lib, config, ... }:.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| { ... }: | |
| { pkgs, lib, config, ... }: |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@darwin/hosts/shared/media.nix` at line 1, Replace the generic module
signature "{ ... }:" with an explicit destructured attribute set (for example "{
pkgs, lib, config, ... }:"), updating the function parameter list in this
module's top-level signature so it follows the repo convention and makes
required inputs (pkgs, lib, config) explicit.
| @@ -1,5 +1,3 @@ | |||
| { pkgs, lib, ... }: | |||
| { ... }: | |||
There was a problem hiding this comment.
Use explicit module arg destructuring for host modules.
Line 1 currently uses { ... }:. Please switch to the repo-standard explicit destructuring form for consistency.
Suggested change
-{ ... }:
+{ pkgs, lib, config, ... }:As per coding guidelines: Use attribute sets with destructuring in function signatures: { pkgs, lib, config, ... }:.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| { ... }: | |
| { pkgs, lib, config, ... }: |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@darwin/hosts/spaceblack.nix` at line 1, The module currently accepts a
catch-all argument set "{ ... }:"; change it to explicit module arg
destructuring like "{ pkgs, lib, config, ... }:" so the host module exposes
standard attributes (pkgs, lib, config) per repo convention; update the function
signature in the top-level of spaceblack.nix to replace "{ ... }:" with the
explicit destructured form and keep the trailing "...": to preserve additional
args.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
flake.nix (1)
133-133: Update the CI runner comment to avoid drift.Line 133 says “linux runners”, but current workflow context indicates macOS-only CI. Rewording this comment will prevent future confusion.
Based on learnings: In csamuel's dotfiles repository, the CI workflow (.github/workflows/ci.yml) uses exclusively macos-15 runners for all jobs, so only aarch64-darwin devShell packages are relevant for CI functionality.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@flake.nix` at line 133, Update the stale comment "# For CI lint on linux runners" to reflect the actual CI platform used; replace it with a concise macOS-specific note such as "# For CI lint on macOS runners (macos-15)" so the comment next to the CI-related devShell/package declarations accurately documents that the workflow uses macos-15 runners and only aarch64-darwin packages are relevant for CI.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@flake.nix`:
- Line 133: Update the stale comment "# For CI lint on linux runners" to reflect
the actual CI platform used; replace it with a concise macOS-specific note such
as "# For CI lint on macOS runners (macos-15)" so the comment next to the
CI-related devShell/package declarations accurately documents that the workflow
uses macos-15 runners and only aarch64-darwin packages are relevant for CI.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 15181c5d-4643-43b8-b842-39b2b9739cbf
📒 Files selected for processing (4)
darwin/modules/determinate-nix.nixflake.nixhome-manager/default.nixhome-manager/p10k/p10k.zsh
🚧 Files skipped from review as they are similar to previous changes (1)
- darwin/modules/determinate-nix.nix
1 participant
Summary
flake checkcoverage fortreefmtanddeadnixtrusted-usersworkaround into its own moduleniland standardize onnixdAGENTS.mdto centernix flake checkin the workflowSummary by CodeRabbit
New Features
Documentation
Chores
Refactor