-
Notifications
You must be signed in to change notification settings - Fork 5
Re-work flake outputs #218
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughPR refactors Nix configuration files: reformats import expressions in default.nix and shell.nix, and substantially restructures flake.nix by updating the outputs function signature to be explicit, importing lib from nixpkgs, reorganizing devShell and packages to be per-system, and renaming system-specific package derivations. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
flake.nix (2)
29-45: Consider updating todevShellsfor consistency.While the current
devShelloutput works, consider aligning with the modern flake schema for consistency with thepackagesrestructuring:- devShell = forEachSupportedSystem ( + devShells = forEachSupportedSystem ( { system, pkgs }: - pkgs.mkShell { - name = "bootspec"; - - packages = with pkgs; [ - cargo - rustc - clippy - codespell - nixpkgs-fmt - rustfmt - jsonschema # provides the jv tool - json-schema-for-humans # provides the generate-schema-doc tool - ]; + { + default = pkgs.mkShell { + name = "bootspec"; + + packages = with pkgs; [ + cargo + rustc + clippy + codespell + nixpkgs-fmt + rustfmt + jsonschema # provides the jv tool + json-schema-for-humans # provides the generate-schema-doc tool + ]; + }; } );This would mirror the
packages.<system>.defaultpattern and align with the modern flake schema.
47-60: Excellent packages restructuring that addresses the PR objective!The new per-system
packagesstructure withdefaultandbootspecproperly replaces the deprecateddefaultPackagepattern. The use ofself.packages.${system}.bootspecfor the default is idiomatic and avoids duplication.Minor simplification opportunity on line 55:
- src = inputs.self; + src = self;Since
selfis already in scope from the function signature{ self, ... }@inputs, using it directly is more concise and idiomatic.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
default.nix(1 hunks)flake.nix(2 hunks)shell.nix(1 hunks)synthesize/integration-test-cases/21.11-specialisations.nix(1 hunks)
⏰ 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). (5)
- GitHub Check: ValidateJsonSchema
- GitHub Check: SynthesizeIntegration
- GitHub Check: NixFormatting
- GitHub Check: format
- GitHub Check: build
🔇 Additional comments (5)
shell.nix (1)
1-4: LGTM! Clean formatting improvement.The reformatted import expression is more concise while maintaining the same functionality.
default.nix (1)
1-4: LGTM! Consistent formatting with shell.nix.The import expression formatting is clean and matches the style used in shell.nix.
synthesize/integration-test-cases/21.11-specialisations.nix (1)
11-18: LGTM! Improved readability.The multi-line formatting of the function expression improves readability without changing behavior.
flake.nix (2)
6-26: Excellent modernization of the flake structure!The explicit outputs function signature,
libimport, andforEachSupportedSystemhelper usinglib.genAttrsare all best practices for modern Nix flakes. This provides a solid foundation for per-system outputs.
34-34: Good update to modern mkShell attribute!The change from
buildInputstopackagesis the recommended attribute formkShellin modern Nix.
I noticed that this was still using
defaultPackageso I fixed that and did some other sprucing-up-type things, like reformatting.Summary by CodeRabbit
Refactor
Style