feat: flake-parts#4
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (25)
WalkthroughMigrates the flake from flake-utils to flake-parts, restructures outputs and per-system exports, replaces many bash wrappers with a Nushell-based scripting framework (shared mkNuScript + scripts/*.nu), adds example configs/tests/validation, updates docs/agents/roadmap, and removes an unused module parameter. Changes
Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
0xgleb
force-pushed
the
feat/flake-parts
branch
from
bce8273 to
3ddacde
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@flake.nix`:
- Around line 97-99: The ragenix package lookup lacks a fallback error unlike
disko, which leads to unclear failures on unsupported systems; update the
ragenix assignment (ragenix = ragenix.packages.${system}.default) to mirror
disko by adding an explicit fallback throw (e.g., ragenix =
ragenix.packages.${system}.default or (throw "ragenix package not available for
${system}")) so that unsupported-system resolution fails with a clear,
actionable message.
- Around line 90-110: The devShells.default currently inherits shellHook from
self.checks.${system}, which can cause evaluation-order recursion; change it to
reference the same-system output via the per-system config argument instead: use
config.checks.git-hooks (the checks.git-hooks output) to inherit shellHook in
devShells.default. Locate the perSystem block and replace the
self.checks.${system}.git-hooks reference with config.checks.git-hooks when
inheriting shellHook so the devShells.default uses the config module output
rather than self to prevent evaluation risks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c2d98ae6-60f1-4e3d-b45c-a17720621eb2
⛔ Files ignored due to path filters (1)
flake.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
flake.nixmodules/static-sites.nix
💤 Files with no reviewable changes (1)
- modules/static-sites.nix
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Graphite / mergeability_check
🧰 Additional context used
📓 Path-based instructions (1)
*
⚙️ CodeRabbit configuration file
Focus on providing constructive criticism. Whenever you see a suboptimal approach, suggest more idiomatic or robust alternative(s). Flag potential footguns. Suggest FP alternatives to mutable/imperative code. Point out architectural flaws like leaky abstractions, tight coupling, wrong level of abstraction, poor type modeling, over-abstraction, unclear domain boundaries. Code should generally be organized based on business concerns rather than technical aspects - suggest improvements if you find violations. Point out gaps in test coverage but suggest tests that are not too coupled to the implementation and actually test domain invariants and business logic
Files:
flake.nix
🔇 Additional comments (1)
flake.nix (1)
36-88: Clean flake-parts migration and output reshaping.This keeps module/lib/template exports readable and preserves the prior composition intent well.
| perSystem = | ||
| { pkgs, system, ... }: | ||
| let | ||
| hooks = self.lib.mkGitHooks { }; | ||
| in | ||
| { | ||
| packages = { | ||
| disko = disko.packages.${system}.default or (throw "disko package not available for ${system}"); | ||
| ragenix = ragenix.packages.${system}.default; | ||
| }; | ||
|
|
||
| checks.git-hooks = git-hooks.lib.${system}.run { | ||
| inherit hooks; | ||
| src = self; | ||
| }; | ||
|
|
||
| formatter = pkgs.nixfmt; | ||
| formatter = pkgs.nixfmt; | ||
|
|
||
| devShells.default = pkgs.mkShell { | ||
| inherit (self.checks.${system}.git-hooks) shellHook; | ||
| packages = [ | ||
| pkgs.nixfmt | ||
| pkgs.deadnix | ||
| pkgs.taplo | ||
| ]; | ||
| devShells.default = pkgs.mkShell { | ||
| inherit (self.checks.${system}.git-hooks) shellHook; | ||
| packages = [ |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🌐 Web query:
In flake-parts perSystem, is referencing config.checks.preferred overself.checks.${system}.for same-system outputs, and are there evaluation pitfalls withself self-references?
💡 Result:
In flake-parts perSystem, both config.checks. and self'.checks. are valid ways to reference locally defined checks for the current system, with no explicit preference stated in the official documentation for same-system outputs. The cheat sheet presents config.packages.hello as "conceptually simpler" and self'.packages.hello as potentially benefiting from future evaluation caching [3,5]. self.checks.${system}. is not a standard perSystem module argument; self refers to the top-level flake, while self' is the pre-selected system version provided in perSystem [5,25]. Using raw self in perSystem risks evaluation order issues or infinite recursion, as seen in flake self-references during imports or outPath access [14,16]. No specific pitfalls are documented for self self-references within perSystem beyond general Nix flake recursion risks when accessing unevaluated attributes [13,14,16]. Prefer module arguments like config or self' to avoid them.
Citations:
- 1: https://flake.parts/cheat-sheet
- 2: https://flake.parts/module-arguments
- 3: https://github.com/hercules-ci/flake-parts/blob/main/modules/perSystem.nix
- 4: Cannot access
inputs.self.outPathin top levelimportshercules-ci/flake-parts#148 - 5: Got
infinite recursion encounteredwhen accessingself.outPathin a flake NixOS/nix#8300 - 6: flake inputs cannot follow "self" NixOS/nix#4931
Use config.checks.git-hooks instead of self.checks.${system} to avoid evaluation risks.
Using self.checks.${system} in perSystem risks evaluation order issues and infinite recursion. The idiomatic approach in flake-parts is to reference same-system outputs via the config module argument.
♻️ Proposed refactor
- perSystem =
- { pkgs, system, ... }:
+ perSystem =
+ { config, pkgs, system, ... }:
let
hooks = self.lib.mkGitHooks { };
in
{
@@
devShells.default = pkgs.mkShell {
- inherit (self.checks.${system}.git-hooks) shellHook;
+ inherit (config.checks.git-hooks) shellHook;
packages = [
pkgs.nixfmt
pkgs.deadnix
pkgs.taplo
];
};📝 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.
| perSystem = | |
| { pkgs, system, ... }: | |
| let | |
| hooks = self.lib.mkGitHooks { }; | |
| in | |
| { | |
| packages = { | |
| disko = disko.packages.${system}.default or (throw "disko package not available for ${system}"); | |
| ragenix = ragenix.packages.${system}.default; | |
| }; | |
| checks.git-hooks = git-hooks.lib.${system}.run { | |
| inherit hooks; | |
| src = self; | |
| }; | |
| formatter = pkgs.nixfmt; | |
| formatter = pkgs.nixfmt; | |
| devShells.default = pkgs.mkShell { | |
| inherit (self.checks.${system}.git-hooks) shellHook; | |
| packages = [ | |
| pkgs.nixfmt | |
| pkgs.deadnix | |
| pkgs.taplo | |
| ]; | |
| devShells.default = pkgs.mkShell { | |
| inherit (self.checks.${system}.git-hooks) shellHook; | |
| packages = [ | |
| perSystem = | |
| { config, pkgs, system, ... }: | |
| let | |
| hooks = self.lib.mkGitHooks { }; | |
| in | |
| { | |
| packages = { | |
| disko = disko.packages.${system}.default or (throw "disko package not available for ${system}"); | |
| ragenix = ragenix.packages.${system}.default; | |
| }; | |
| checks.git-hooks = git-hooks.lib.${system}.run { | |
| inherit hooks; | |
| src = self; | |
| }; | |
| formatter = pkgs.nixfmt; | |
| devShells.default = pkgs.mkShell { | |
| inherit (config.checks.git-hooks) shellHook; | |
| packages = [ |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@flake.nix` around lines 90 - 110, The devShells.default currently inherits
shellHook from self.checks.${system}, which can cause evaluation-order
recursion; change it to reference the same-system output via the per-system
config argument instead: use config.checks.git-hooks (the checks.git-hooks
output) to inherit shellHook in devShells.default. Locate the perSystem block
and replace the self.checks.${system}.git-hooks reference with
config.checks.git-hooks when inheriting shellHook so the devShells.default uses
the config module output rather than self to prevent evaluation risks.
| disko = disko.packages.${system}.default or (throw "disko package not available for ${system}"); | ||
| ragenix = ragenix.packages.${system}.default; | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Make ragenix package lookup failure explicit like disko.
disko has a clear fallback error, but ragenix does not. Aligning both gives a more actionable failure on unsupported systems.
🛡️ Proposed robustness tweak
packages = {
disko = disko.packages.${system}.default or (throw "disko package not available for ${system}");
- ragenix = ragenix.packages.${system}.default;
+ ragenix = ragenix.packages.${system}.default or (throw "ragenix package not available for ${system}");
};As per coding guidelines, "Whenever you see a suboptimal approach, suggest more idiomatic or robust alternative(s)."
📝 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.
| disko = disko.packages.${system}.default or (throw "disko package not available for ${system}"); | |
| ragenix = ragenix.packages.${system}.default; | |
| }; | |
| disko = disko.packages.${system}.default or (throw "disko package not available for ${system}"); | |
| ragenix = ragenix.packages.${system}.default or (throw "ragenix package not available for ${system}"); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@flake.nix` around lines 97 - 99, The ragenix package lookup lacks a fallback
error unlike disko, which leads to unclear failures on unsupported systems;
update the ragenix assignment (ragenix = ragenix.packages.${system}.default) to
mirror disko by adding an explicit fallback throw (e.g., ragenix =
ragenix.packages.${system}.default or (throw "ragenix package not available for
${system}")) so that unsupported-system resolution fails with a clear,
actionable message.
1 participant
Summary by CodeRabbit
New Features
Documentation
Chores
Tests