feat(nix): add statix linting and fix all lint errors - #1222
Conversation
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
You do not have enough credits to review this pull request. Please purchase more credits to continue. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (6)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdded a GitHub Actions job Changes
Sequence Diagram(s)sequenceDiagram
participant GH as "GitHub Actions"
participant Repo as "Repository"
participant Runner as "ubuntu-latest runner"
participant Nix as "Nix / statix"
participant Make as "Makefile"
GH->>Repo: workflow triggered (nix)
GH->>Runner: start job `nix-lint`
Runner->>Repo: actions/checkout
Runner->>Nix: install Nix (cachix/install-nix-action)
Runner->>Make: run `make nix-lint`
Make->>Nix: execute `nix run nixpkgs#statix -- check .`
Nix-->>Make: statix results
Make-->>Runner: exit status
Runner-->>GH: job complete (success/failure)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
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 |
Mesa DescriptionTL;DRAdded What changed?
Description generated by Mesa. Update settings |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/lib.nix (1)
64-64: Subtle semantic change in test assertion.The original
== truestrictly asserted the value is the booleantrue. The new truthy check would also pass for other truthy values (e.g., a non-empty string or number). Given thatlib/nixpkgs-config.nixhardcodesallowUnfree = true, this is acceptable, but note that the test now validates "exists and is truthy" rather than "is exactlytrue".If strict boolean validation is desired, consider using
builtins.isBoollike the other tests in this file:if nixpkgsConfig ? allowUnfree && builtins.isBool nixpkgsConfig.allowUnfree && nixpkgsConfig.allowUnfree then🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/lib.nix` at line 64, The test currently checks if nixpkgsConfig ? allowUnfree && nixpkgsConfig.allowUnfree which accepts any truthy value; change the condition to require a strict boolean by adding a builtins.isBool check for nixpkgsConfig.allowUnfree (i.e., ensure builtins.isBool(nixpkgsConfig.allowUnfree) && nixpkgsConfig.allowUnfree) so the test only passes when allowUnfree is exactly true; update the conditional in tests/lib.nix accordingly referencing nixpkgsConfig and allowUnfree.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/lib.nix`:
- Line 64: The test currently checks if nixpkgsConfig ? allowUnfree &&
nixpkgsConfig.allowUnfree which accepts any truthy value; change the condition
to require a strict boolean by adding a builtins.isBool check for
nixpkgsConfig.allowUnfree (i.e., ensure
builtins.isBool(nixpkgsConfig.allowUnfree) && nixpkgsConfig.allowUnfree) so the
test only passes when allowUnfree is exactly true; update the conditional in
tests/lib.nix accordingly referencing nixpkgsConfig and allowUnfree.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1d75d843-e1b2-4372-abcf-d9627df1720e
📒 Files selected for processing (23)
.github/workflows/nix.ymlMakefileconfig/keyd/default.nixconfig/rofi/default.nixconfig/tmuxinator/default.nixdevenv.nixhome-manager/modules/cargo-globals/default.nixhome-manager/modules/openclaw/default.nixhome-manager/modules/yek/default.nixhome-manager/programs/fish/default.nixhome-manager/services/cliproxyapi/default.nixhome-manager/services/docker-postgres/default.nixhome-manager/services/docker/default.nixhosts/darwin/default.nixhosts/linux/default.nixhosts/nixos/default.nixnamed-hosts/galactica/default.nixnamed-hosts/kyber/default.nixnamed-hosts/matic/default.nixnamed-hosts/matic/falcon.nixoverlays/default.nixstatix.tomltests/lib.nix
There was a problem hiding this comment.
Pull request overview
This PR integrates statix linting into the repo’s Nix workflow (local make + CI) and applies repo-wide Nix refactors to satisfy the new linter rules.
Changes:
- Add
statix.tomlconfiguration and amake nix-linttarget, and wire it intomake check. - Add a
nix-lintjob to the GitHub Actions Nix workflow and gate it via the existing “alls-green” check. - Apply statix-driven cleanups across multiple Nix files (e.g.,
inherit, boolean simplifications, unused arg patterns).
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/lib.nix | Simplifies allowUnfree boolean check per statix guidance. |
| statix.toml | Adds statix configuration (disable repeated_keys, set version, ignore .direnv). |
| overlays/default.nix | Refactors attribute assignments to inherit for statix compliance. |
| named-hosts/matic/falcon.nix | Uses inherit in replaceVars substitutions. |
| named-hosts/matic/default.nix | Refactors args/secrets mapping to use inherit. |
| named-hosts/kyber/default.nix | Uses inherit for lib, username, and secrets mapping. |
| named-hosts/galactica/default.nix | Uses inherit for specialArgs and secrets mapping. |
| hosts/nixos/default.nix | Uses inherit for lib and pkgs when passing args to HM. |
| hosts/linux/default.nix | Uses inherit for lib and username. |
| hosts/darwin/default.nix | Replaces unused module arg pattern with _: and uses inherit for lib. |
| home-manager/services/docker/default.nix | Uses inherit in replaceVars substitutions. |
| home-manager/services/docker-postgres/default.nix | Uses inherit in replaceVars substitutions. |
| home-manager/services/cliproxyapi/default.nix | Uses inherit in replaceVars substitutions. |
| home-manager/programs/fish/default.nix | Refactors fish plugin src fields to use inherit. |
| home-manager/modules/yek/default.nix | Uses inherit in replaceVars substitutions. |
| home-manager/modules/openclaw/default.nix | Removes unnecessary parentheses in mkIf. |
| home-manager/modules/cargo-globals/default.nix | Uses inherit for isDarwin. |
| devenv.nix | Uses inherit in replaceVars substitutions (fishtape wrapper). |
| config/tmuxinator/default.nix | Replaces unused arg pattern with _:. |
| config/rofi/default.nix | Replaces unused arg pattern with _:. |
| config/keyd/default.nix | Replaces unused arg pattern with _:. |
| Makefile | Adds nix-lint target and runs it as part of check. |
| .github/workflows/nix.yml | Adds nix-lint CI job and includes it in the overall Nix workflow gate. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| .PHONY: nix-lint | ||
| nix-lint: ## Lint Nix files with statix. | ||
| @echo "🔍 Linting Nix files with statix..." | ||
| @$(NIX_EXEC) run nixpkgs#statix -- check . |
There was a problem hiding this comment.
nix-lint runs nix run nixpkgs#statix without $(NIX_FLAGS). Other Nix commands in this Makefile pass $(NIX_FLAGS) to ensure flakes/nix-command are enabled and cache options apply; without it, make nix-lint can fail on machines where those features aren’t enabled globally. Pass $(NIX_FLAGS) (and keep the invocation consistent with the other nix run targets).
| @$(NIX_EXEC) run nixpkgs#statix -- check . | |
| @$(NIX_EXEC) run $(NIX_FLAGS) nixpkgs#statix -- check . |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request is a great improvement, adding statix linting and fixing all associated issues across the codebase. The Nix code is now more consistent and idiomatic. I've added a few minor suggestions to further enhance conciseness and consistency in a couple of files, in line with the goals of this PR.
| builtins.readFile ( | ||
| pkgs.replaceVars ./scripts/docker-start.sh { | ||
| bash = pkgs.bash; | ||
| inherit (pkgs) bash; |
There was a problem hiding this comment.
For conciseness, you can combine this inherit with the one for docker on line 38. I've added a suggestion on line 38 to remove the now-redundant line.
inherit (pkgs) bash docker;
| inherit (pkgs) bash; | ||
| start_script = startScript; | ||
| docker = pkgs.docker; | ||
| inherit (pkgs) docker; |
There was a problem hiding this comment.
This line is now redundant and can be removed, as it has been combined into the inherit statement on line 36.
| builtins.readFile ( | ||
| pkgs.replaceVars ./start-postgres-wrapper.sh { | ||
| bash = pkgs.bash; | ||
| inherit (pkgs) bash; |
There was a problem hiding this comment.
For conciseness, you can combine this inherit with the one for docker on line 14. I've added a suggestion on line 14 to remove the now-redundant line.
inherit (pkgs) bash docker;
| inherit (pkgs) bash; | ||
| start_script = startScript; | ||
| docker = pkgs.docker; | ||
| inherit (pkgs) docker; |
There was a problem hiding this comment.
This line is now redundant and can be removed, as it has been combined into the inherit statement on line 12.
| inherit (nixpkgs) lib; | ||
| inherit pkgs; |
There was a problem hiding this comment.
To improve consistency with other parts of your configuration, you can inherit pkgs first and then lib from pkgs. Since pkgs is derived from nixpkgs, pkgs.lib is equivalent to nixpkgs.lib here.
inherit pkgs;
inherit (pkgs) lib;
| inherit (inputs.nixpkgs) lib; | ||
| inherit pkgs; |
There was a problem hiding this comment.
To improve consistency, consider inheriting pkgs first, and then lib from pkgs. Since pkgs is derived from inputs.nixpkgs, this is functionally equivalent and slightly more idiomatic.
inherit pkgs;
inherit (pkgs) lib;
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
statixnix linter integration withmake nix-linttarget andstatix.tomlconfig (W20repeated_keysdisabled as it's idiomatic in NixOS/home-manager)inheritinstead of manual assignment (W03/W04), remove unnecessary boolean comparisons (W01), remove useless parentheses (W08), replace empty patterns with_(W10)nix-lintGitHub Actions job to the Nix workflow with gate checkTest plan
nix run nixpkgs#statix -- check .passes clean (exit 0)nix-lintCI job passes on PR🤖 Generated with Claude Code
Summary by cubic
Adds
statixlinting with amake nix-linttarget and a CI job, fixes all Nix lint issues, and applies the project formatter. This enforces consistent Nix style and adds a gate to prevent regressions.New Features
make nix-lint(runsnixpkgs#statix) and included it inmake check.nix-lintjob to.github/workflows/nix.ymland included it in the workflow gate.statix.toml(disablesrepeated_keys, setsnix_version, ignores.direnv).Refactors
statixfindings: useinherit/inherit (scope), simplify booleans, remove extra parens, use_:for unused args, and replace empty patterns with_.Written for commit ddd8af5. Summary will update on new commits.