Update package configurations and fix Zsh settings - #345
Conversation
…ptional Linux packages
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a dedicated home-manager module for btop, removes btop from the package list, adds/adjusts several packages with Linux- and CI-guards, enables Zsh and extends direnv integrations, and simplifies Free Disk Space inputs in multiple GitHub Actions workflows. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Repo
participant HM as Home-Manager
participant CI as GitHub Actions
participant User
rect rgba(102,153,204,0.06)
Repo->>Repo: add programs/btop/default.nix\nmodify packages/default.nix (remove btop, add packages, add isCI)
Repo->>Repo: update programs/default.nix and direnv/zsh modules
Repo->>Repo: adjust Free Disk Space steps in workflows
end
rect rgba(102,204,153,0.06)
User->>HM: home-manager build/apply
HM->>HM: load programs registry (includes new btop module)
HM-->>User: apply btop configuration and program settings
end
rect rgba(204,153,102,0.06)
CI->>Repo: checkout
CI->>CI: run Free Disk Space step (tool-cache: false)
CI->>CI: install Nix and continue jobs
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
Possibly related PRs
Suggested labels
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 |
Summary of ChangesHello @shunkakinoki, 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 refines the system's package management and shell configurations. It updates the list of installed packages, introduces a detailed configuration for the "btop" system monitor, and corrects Zsh settings to ensure proper functionality. Additionally, "direnv" integration with various shells has been explicitly defined. Highlights
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. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Mesa DescriptionTL;DRThis PR updates Home Manager configurations by adding What changed?
Description generated by Mesa. Update settings |
![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 updates package configurations, adds btop as a home-manager program, and enables zsh. The changes are generally good, but I've found a few issues. There are critical syntax errors in the new btop configuration that will prevent the configuration from building. I've also noticed a minor package sorting issue and a redundancy in the shell integration for direnv. My review includes suggestions to fix these issues.
| custom_cpu_name = "; | ||
| disks_filter = "; | ||
| mem_graphs = true; | ||
| mem_below_net = false; | ||
| zfs_arc_cached = true; | ||
| show_swap = true; | ||
| swap_disk = true; | ||
| show_disks = true; | ||
| only_physical = true; | ||
| use_fstab = true; | ||
| zfs_hide_datasets = false; | ||
| disk_free_priv = false; | ||
| show_io_stat = true; | ||
| io_mode = false; | ||
| io_graph_combined = false; | ||
| io_graph_speeds = "; | ||
| net_download = 100; | ||
| net_upload = 100; | ||
| net_auto = true; | ||
| net_sync = true; | ||
| net_iface = "; |
There was a problem hiding this comment.
There are several syntax errors in this block. The string values for custom_cpu_name, disks_filter, io_graph_speeds, and net_iface are incorrectly terminated with a semicolon (;) instead of a double quote ("). This will cause the Nix evaluation to fail.
custom_cpu_name = "";
disks_filter = "";
mem_graphs = true;
mem_below_net = false;
zfs_arc_cached = true;
show_swap = true;
swap_disk = true;
show_disks = true;
only_physical = true;
use_fstab = true;
zfs_hide_datasets = false;
disk_free_priv = false;
show_io_stat = true;
io_mode = false;
io_graph_combined = false;
io_graph_speeds = "";
net_download = 100;
net_upload = 100;
net_auto = true;
net_sync = true;
net_iface = "";
| powertop | ||
| opencode |
There was a problem hiding this comment.
To improve maintainability, the packages in this list should be kept in alphabetical order. opencode should come before powertop.
opencode
powertop
| enableBashIntegration = true; | ||
| # enableFishIntegration = true; | ||
| enableZshIntegration = true; | ||
| nix-direnv.enable = true; |
There was a problem hiding this comment.
Enabling enableBashIntegration and enableZshIntegration makes the manual direnv hooks in your bash and zsh configurations redundant. To keep the configuration clean and avoid potential issues, you should remove eval \"$(direnv hook bash)\" from home-manager/programs/bash/default.nix and eval \"$(direnv hook zsh)\" from home-manager/programs/zsh/default.nix.
| }; | ||
|
|
||
| initExtra = '' | ||
| initContent = '' |
There was a problem hiding this comment.
The initExtra option is deprecated in home-manager. You have correctly changed it to initContent. This is a good improvement for maintaining compatibility with future home-manager versions.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| custom_cpu_name = "; | ||
| disks_filter = "; |
There was a problem hiding this comment.
The new btop module sets several options to "; (custom_cpu_name, disks_filter, io_graph_speeds, net_iface). Each is missing the closing quote for an empty string, so the file will not parse and Home Manager evaluation fails before any configuration can be applied. Replace them with valid string values such as ""; or remove the lines.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull Request Overview
This PR updates package configurations and fixes Zsh settings. The main changes include enabling Zsh, adding btop as a configured program instead of just a package, and adding several new Linux-specific packages. However, there are critical syntax errors that need to be fixed before this PR can be merged.
- Enable Zsh and update its initialization configuration
- Add btop as a configured program with comprehensive settings
- Add new packages (clipse, gnumake) and Linux-specific packages (blueberry, chromium, ffmpeg, github-desktop, powertop, signal-desktop, vlc)
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| home-manager/programs/zsh/default.nix | Enables Zsh and updates initialization attribute (contains bug) |
| home-manager/programs/direnv/default.nix | Adds shell integration settings for direnv |
| home-manager/programs/default.nix | Imports and exports btop program configuration |
| home-manager/programs/btop/default.nix | Adds comprehensive btop configuration (contains syntax errors) |
| home-manager/packages/default.nix | Moves btop to programs, adds new default and Linux-specific packages |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }; | ||
|
|
||
| initExtra = '' | ||
| initContent = '' |
There was a problem hiding this comment.
The attribute name initContent is incorrect for home-manager's zsh module. The correct attribute is initExtra. This change will cause a configuration error as initContent is not a recognized option for programs.zsh in home-manager.
Change this back to:
initExtra = ''| initContent = '' | |
| initExtra = '' |
| show_cpu_freq = true; | ||
| clock_format = "%X"; | ||
| background_update = true; | ||
| custom_cpu_name = "; |
There was a problem hiding this comment.
Syntax error: The string value is not properly closed. This line should be:
custom_cpu_name = "";| custom_cpu_name = "; | |
| custom_cpu_name = ""; |
| clock_format = "%X"; | ||
| background_update = true; | ||
| custom_cpu_name = "; | ||
| disks_filter = "; |
There was a problem hiding this comment.
Syntax error: The string value is not properly closed. This line should be:
disks_filter = "";| disks_filter = "; | |
| disks_filter = ""; |
| show_io_stat = true; | ||
| io_mode = false; | ||
| io_graph_combined = false; | ||
| io_graph_speeds = "; |
There was a problem hiding this comment.
Syntax error: The string value is not properly closed. This line should be:
io_graph_speeds = "";| io_graph_speeds = "; | |
| io_graph_speeds = ""; |
| net_upload = 100; | ||
| net_auto = true; | ||
| net_sync = true; | ||
| net_iface = "; |
There was a problem hiding this comment.
Syntax error: The string value is not properly closed. This line should be:
net_iface = "";| net_iface = "; | |
| net_iface = ""; |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
home-manager/programs/zsh/default.nix (1)
25-25: Remove redundant direnv hook - already handled by enableZshIntegration.With
enableZshIntegration = trueset inhome-manager/programs/direnv/default.nix(line 6), home-manager automatically injects the direnv hook. The manualeval "$(direnv hook zsh)"here creates a duplicate initialization.Apply this diff to remove the redundant hook:
eval "$(sheldon source)" - eval "$(direnv hook zsh)" eval "$(/opt/homebrew/bin/brew shellenv)"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
home-manager/packages/default.nix(3 hunks)home-manager/programs/btop/default.nix(1 hunks)home-manager/programs/default.nix(2 hunks)home-manager/programs/direnv/default.nix(1 hunks)home-manager/programs/zsh/default.nix(2 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). (11)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: Agent
- GitHub Check: Mesa Review
- GitHub Check: e2e-run (NixOS, ubuntu-latest)
- GitHub Check: e2e-run (MacOS, macos-latest)
- GitHub Check: e2e-run (Ubuntu, ubuntu-latest)
- GitHub Check: docker-build-push (linux/arm64, arm64, ubuntu-24.04-arm)
- GitHub Check: docker-build-push (linux/amd64, amd64, ubuntu-latest)
- GitHub Check: nix-nixos
- GitHub Check: nix-linux
- GitHub Check: nix-darwin
🔇 Additional comments (5)
home-manager/programs/zsh/default.nix (1)
5-5: Enabling Zsh looks good.The change to enable Zsh by default aligns with the PR objectives.
home-manager/programs/direnv/default.nix (1)
4-7: LGTM! Direnv integrations configured properly.The Bash and Zsh integrations along with nix-direnv support are correctly configured using standard home-manager attributes.
home-manager/programs/default.nix (1)
10-10: LGTM! Module import follows the established pattern.The btop module is properly imported and exported, maintaining alphabetical order and consistency with other program modules.
Also applies to: 39-39
home-manager/packages/default.nix (2)
17-17: Static package additions look good.The additions of
clipseandgnumaketo the static package list are properly placed.Also applies to: 34-34
73-87: I need to gather more information about the repository context and verify nixpkgs package availability. Let me examine the file and search for package information.Let me examine the repository structure to see the actual file context:
Based on my web search findings, I can now provide the final response:
Review comment is addressed and no further action needed.
The
blueberrypackage is available in nixpkgs, and theopencodepackage is also declared in nixpkgs. Additionally, theclaude-codepackage is provided by upstream nixpkgs with a standard update cycle.The other packages in the code snippet—including
chromium,signal-desktop,docker,docker-compose,ffmpeg, and others—are all established, widely-used packages that are actively maintained in nixpkgs. While package availability can vary across different nixpkgs versions and channels (which is a normal characteristic of how nixpkgs versioning works), all the packages referenced in the snippet are documented as available in current nixpkgs releases.The Linux-conditional placement using
lib.optionals stdenv.isLinuxis the correct approach for platform-specific packages.
| { | ||
| # From: https://github.com/henrysipp/omarchy-nix/blob/308e0f85a0deb820c01cfbe1b4faee1daab4da12/modules/home-manager/btop.nix | ||
| programs.btop = { | ||
| enable = true; | ||
| settings = { | ||
| theme_background = false; | ||
| truecolor = true; | ||
| force_tty = false; | ||
| vim_keys = true; | ||
| rounded_corners = true; | ||
| graph_symbol = "braille"; | ||
| graph_symbol_cpu = "default"; | ||
| graph_symbol_mem = "default"; | ||
| graph_symbol_net = "default"; | ||
| graph_symbol_proc = "default"; | ||
| shown_boxes = "cpu mem net proc"; | ||
| update_ms = 2000; | ||
| proc_sorting = "cpu lazy"; | ||
| proc_reversed = false; | ||
| proc_tree = false; | ||
| proc_colors = true; | ||
| proc_gradient = false; | ||
| proc_per_core = false; | ||
| proc_mem_bytes = true; | ||
| proc_cpu_graphs = true; | ||
| proc_info_smaps = false; | ||
| proc_left = false; | ||
| cpu_graph_upper = "total"; | ||
| cpu_graph_lower = "total"; | ||
| cpu_invert_lower = true; | ||
| cpu_single_graph = false; | ||
| cpu_bottom = false; | ||
| show_uptime = true; | ||
| check_temp = true; | ||
| cpu_sensor = "Auto"; | ||
| show_coretemp = true; | ||
| cpu_core_map = ""; | ||
| temp_scale = "celsius"; | ||
| base_10_sizes = false; | ||
| show_cpu_freq = true; | ||
| clock_format = "%X"; | ||
| background_update = true; | ||
| custom_cpu_name = "; | ||
| disks_filter = "; | ||
| mem_graphs = true; | ||
| mem_below_net = false; | ||
| zfs_arc_cached = true; | ||
| show_swap = true; | ||
| swap_disk = true; | ||
| show_disks = true; | ||
| only_physical = true; | ||
| use_fstab = true; | ||
| zfs_hide_datasets = false; | ||
| disk_free_priv = false; | ||
| show_io_stat = true; | ||
| io_mode = false; | ||
| io_graph_combined = false; | ||
| io_graph_speeds = "; | ||
| net_download = 100; | ||
| net_upload = 100; | ||
| net_auto = true; | ||
| net_sync = true; | ||
| net_iface = "; | ||
| show_battery = true; | ||
| selected_battery = "Auto"; | ||
| log_level = "WARNING"; | ||
| }; | ||
| }; |
There was a problem hiding this comment.
❓ Verification inconclusive
Consider verifying btop configuration options.
The btop configuration contains extensive settings copied from a reference repository. While the structure looks correct (aside from the string literal issues), you may want to verify these settings match your preferred btop behavior.
Run this script to check the btop package and review available configuration options:
🏁 Script executed:
#!/bin/bash
# Description: Verify btop is available in nixpkgs and check for documentation
# Check if btop exists in nixpkgs
nix-env -qaP -A nixpkgs.btop
# Look for btop home-manager module documentation
fd -e nix btop | head -20Length of output: 45
I'll search for documentation on Nix syntax to verify the string literal issues are indeed errors:
Fix critical syntax errors in btop configuration.
The btop settings contain unclosed string literals that will cause Nix evaluation to fail. In Nix, strings are enclosed between double quotes, and each of the following assignments is missing the opening quote:
custom_cpu_name = ";(should becustom_cpu_name = "";)disks_filter = ";(should bedisks_filter = "";)io_graph_speeds = ";(should beio_graph_speeds = "";)net_iface = ";(should benet_iface = "";)
These errors will prevent the module from loading. If these settings should have specific values, provide them; otherwise, use empty strings as shown above.
🤖 Prompt for AI Agents
In home-manager/programs/btop/default.nix around lines 1 to 68, several btop
string assignments have unclosed/missing quotes (custom_cpu_name, disks_filter,
io_graph_speeds, net_iface) which will break Nix evaluation; fix by closing the
string literals—either set them to proper values or to empty strings (e.g., ""),
ensuring each assignment uses a valid quoted string.
| custom_cpu_name = "; | ||
| disks_filter = "; |
There was a problem hiding this comment.
Fix malformed string literals - missing opening quotes.
Several string settings have syntax errors with only a closing quote and semicolon. These will cause Nix evaluation to fail.
Apply this diff to fix the malformed strings:
clock_format = "%X";
background_update = true;
- custom_cpu_name = ";
- disks_filter = ";
+ custom_cpu_name = "";
+ disks_filter = "";
mem_graphs = true; io_mode = false;
io_graph_combined = false;
- io_graph_speeds = ";
+ io_graph_speeds = "";
net_download = 100; net_auto = true;
net_sync = true;
- net_iface = ";
+ net_iface = "";
show_battery = true;Also applies to: 58-58, 63-63
🤖 Prompt for AI Agents
In home-manager/programs/btop/default.nix around lines 43-44 (and also lines 58
and 63), several string assignments are malformed: they only have a closing
quote and semicolon (e.g. custom_cpu_name = ";). Fix each by providing the
missing opening quote and a valid string value (or an empty string ""), ensuring
the assignment uses proper Nix string syntax: key = "value";; update
custom_cpu_name, disks_filter, and the settings at lines 58 and 63 to use
complete quotes and valid values.
| }; | ||
|
|
||
| initExtra = '' | ||
| initContent = '' |
There was a problem hiding this comment.
Incorrect attribute name - should be initExtra, not initContent.
The standard home-manager Zsh module attribute for shell initialization is initExtra, not initContent. This will cause the configuration to fail.
Apply this diff to fix the attribute name:
- initContent = ''
+ initExtra = ''📝 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.
| initContent = '' | |
| initExtra = '' |
🤖 Prompt for AI Agents
In home-manager/programs/zsh/default.nix around line 20, the attribute is
incorrectly named initContent; change this attribute to initExtra so it uses the
standard home-manager Zsh module option. Update the attribute key from
initContent to initExtra and ensure any surrounding quoting/indenting remains
consistent with the file.
![mesa-dot-dev[bot]](https://avatars.githubusercontent.com/in/1050077?s=60&v=4){kind=small}
There was a problem hiding this comment.
Performed full review of 7aa6db8...ae56d67
Analysis
-
CRITICAL: Breaking Zsh Configuration - The change from
initExtratoinitContentuses a non-existent option in Home Manager's Zsh module that will cause build failures. -
Inconsistent Multi-Shell Strategy - The codebase enables Bash, Fish, and Zsh simultaneously without clear documentation or rationale, creating potential conflicts in login environments and shell integration priorities.
-
Direnv Integration Inconsistency - Direnv integration is enabled for Bash and Zsh but commented out for Fish, despite Fish appearing to be the primary shell based on its extensive configuration.
-
Package Management Redundancy - Programs like btop, bat, and delta exist in both the packages list and as dedicated program modules, creating confusion about where packages are officially managed and potential maintenance issues.
Tip
Help
Slash Commands:
/review- Request a full code review/review latest- Review only changes since the last review/describe- Generate PR description. This will update the PR body or issue comment depending on your configuration/help- Get help with Mesa commands and configuration options
5 files reviewed | 2 comments | Edit Agent Settings • Read Docs
| }; | ||
|
|
||
| initExtra = '' | ||
| initContent = '' |
There was a problem hiding this comment.
The initContent option does not exist in Home Manager's programs.zsh module. This should be initExtra. Home Manager's zsh module only supports initExtra for custom initialization code, not initContent. This change will cause a configuration build error.
Agent: 🏛 Architecture •
| { | ||
| programs.zsh = { | ||
| enable = false; | ||
| enable = true; |
There was a problem hiding this comment.
Enabling Zsh alongside Fish (which is already extensively configured with functions, completions, and shell integrations) creates potential conflicts. Multiple enabled shells can cause login shell priority issues, conflicting shell integrations, and environment variable initialization order problems. Consider clarifying which shell is primary or documenting if this is a migration path from Fish to Zsh.
Agent: 🏛 Architecture •
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/nix.yml (2)
83-92: Consider consolidating disk-space freeing logic to avoid duplication.The identical
Free Disk Space (Ubuntu)step appears in bothnix-linuxandnix-nixosjobs. If this pattern needs to be replicated across other Ubuntu jobs or becomes standard, consider extracting it as a reusable workflow or composite action to reduce duplication and maintenance burden.
59-68: The action exists and is correctly configured; consider disk-space optimization for other Ubuntu build jobs.The
jlumbroso/free-disk-space@v1.3.1action is valid, and the configuration matches the action's input schema. However,nix-flakeandnix-formatare resource-intensive jobs (300-minute timeout, Nix operations) running onubuntu-latestthat lack the disk-freeing step present innix-linuxandnix-nixos. For consistency and robustness, these jobs could benefit from the same disk-space optimization—though this is optional if disk space constraints are specific to the build jobs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/nix.yml(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). (8)
- GitHub Check: docker-build-push (linux/amd64, amd64, ubuntu-latest)
- GitHub Check: docker-build-push (linux/arm64, arm64, ubuntu-24.04-arm)
- GitHub Check: nix-nixos
- GitHub Check: nix-linux
- GitHub Check: nix-darwin
- GitHub Check: e2e-run (MacOS, macos-latest)
- GitHub Check: e2e-run (NixOS, ubuntu-latest)
- GitHub Check: e2e-run (Ubuntu, ubuntu-latest)
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
5 issues found across 5 files
Prompt for AI agents (all 5 issues)
Understand the root cause of the following 5 issues and fix them.
<file name="home-manager/programs/zsh/default.nix">
<violation number="1" location="home-manager/programs/zsh/default.nix:20">
Using `initContent` overwrites the default Zsh setup provided by Home Manager, which breaks the automatic integration for other programs. The integrations for `direnv`, `atuin`, and `lsd` will stop working. You should use `initExtra` to append to the configuration instead of replacing it.</violation>
</file>
<file name="home-manager/programs/btop/default.nix">
<violation number="1" location="home-manager/programs/btop/default.nix:43">
The string literal here is missing its closing quote, so the Nix module will fail to parse. Please close the string.</violation>
<violation number="2" location="home-manager/programs/btop/default.nix:44">
This assignment is missing the closing quote, leaving the string unterminated and breaking the configuration. Close the string literal.</violation>
<violation number="3" location="home-manager/programs/btop/default.nix:58">
The new `io_graph_speeds` value is missing the terminating quote, so the configuration will not parse. Close the string literal.</violation>
<violation number="4" location="home-manager/programs/btop/default.nix:63">
The `net_iface` assignment is missing the closing quote; as written it will break the module. Close the string literal.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| }; | ||
|
|
||
| initExtra = '' | ||
| initContent = '' |
There was a problem hiding this comment.
Using initContent overwrites the default Zsh setup provided by Home Manager, which breaks the automatic integration for other programs. The integrations for direnv, atuin, and lsd will stop working. You should use initExtra to append to the configuration instead of replacing it.
Prompt for AI agents
Address the following comment on home-manager/programs/zsh/default.nix at line 20:
<comment>Using `initContent` overwrites the default Zsh setup provided by Home Manager, which breaks the automatic integration for other programs. The integrations for `direnv`, `atuin`, and `lsd` will stop working. You should use `initExtra` to append to the configuration instead of replacing it.</comment>
<file context>
@@ -17,7 +17,7 @@
};
- initExtra = ''
+ initContent = ''
# Initialize Starship
eval "$(starship init zsh)"
</file context>
| show_cpu_freq = true; | ||
| clock_format = "%X"; | ||
| background_update = true; | ||
| custom_cpu_name = "; |
There was a problem hiding this comment.
The string literal here is missing its closing quote, so the Nix module will fail to parse. Please close the string.
Prompt for AI agents
Address the following comment on home-manager/programs/btop/default.nix at line 43:
<comment>The string literal here is missing its closing quote, so the Nix module will fail to parse. Please close the string.</comment>
<file context>
@@ -0,0 +1,69 @@
+ show_cpu_freq = true;
+ clock_format = "%X";
+ background_update = true;
+ custom_cpu_name = ";
+ disks_filter = ";
+ mem_graphs = true;
</file context>
| custom_cpu_name = "; | |
| custom_cpu_name = ""; |
| net_upload = 100; | ||
| net_auto = true; | ||
| net_sync = true; | ||
| net_iface = "; |
There was a problem hiding this comment.
The net_iface assignment is missing the closing quote; as written it will break the module. Close the string literal.
Prompt for AI agents
Address the following comment on home-manager/programs/btop/default.nix at line 63:
<comment>The `net_iface` assignment is missing the closing quote; as written it will break the module. Close the string literal.</comment>
<file context>
@@ -0,0 +1,69 @@
+ net_upload = 100;
+ net_auto = true;
+ net_sync = true;
+ net_iface = ";
+ show_battery = true;
+ selected_battery = "Auto";
</file context>
| net_iface = "; | |
| net_iface = ""; |
| show_io_stat = true; | ||
| io_mode = false; | ||
| io_graph_combined = false; | ||
| io_graph_speeds = "; |
There was a problem hiding this comment.
The new io_graph_speeds value is missing the terminating quote, so the configuration will not parse. Close the string literal.
Prompt for AI agents
Address the following comment on home-manager/programs/btop/default.nix at line 58:
<comment>The new `io_graph_speeds` value is missing the terminating quote, so the configuration will not parse. Close the string literal.</comment>
<file context>
@@ -0,0 +1,69 @@
+ show_io_stat = true;
+ io_mode = false;
+ io_graph_combined = false;
+ io_graph_speeds = ";
+ net_download = 100;
+ net_upload = 100;
</file context>
| io_graph_speeds = "; | |
| io_graph_speeds = ""; |
| clock_format = "%X"; | ||
| background_update = true; | ||
| custom_cpu_name = "; | ||
| disks_filter = "; |
There was a problem hiding this comment.
This assignment is missing the closing quote, leaving the string unterminated and breaking the configuration. Close the string literal.
Prompt for AI agents
Address the following comment on home-manager/programs/btop/default.nix at line 44:
<comment>This assignment is missing the closing quote, leaving the string unterminated and breaking the configuration. Close the string literal.</comment>
<file context>
@@ -0,0 +1,69 @@
+ clock_format = "%X";
+ background_update = true;
+ custom_cpu_name = ";
+ disks_filter = ";
+ mem_graphs = true;
+ mem_below_net = false;
</file context>
| disks_filter = "; | |
| disks_filter = ""; |
…d on CI environment
…ce action in Docker, E2E, and Nix workflows
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
.github/workflows/docker.yml(0 hunks).github/workflows/e2e.yml(0 hunks).github/workflows/nix.yml(1 hunks)home-manager/packages/default.nix(4 hunks)
💤 Files with no reviewable changes (2)
- .github/workflows/docker.yml
- .github/workflows/e2e.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/nix.yml
⏰ 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). (9)
- GitHub Check: Agent
- GitHub Check: e2e-run (MacOS, macos-latest)
- GitHub Check: e2e-run (Ubuntu, ubuntu-latest)
- GitHub Check: e2e-run (NixOS, ubuntu-latest)
- GitHub Check: docker-build-push (linux/arm64, arm64, ubuntu-24.04-arm)
- GitHub Check: docker-build-push (linux/amd64, amd64, ubuntu-latest)
- GitHub Check: nix-linux
- GitHub Check: nix-nixos
- GitHub Check: nix-darwin
🔇 Additional comments (4)
home-manager/packages/default.nix (4)
20-20: LGTM!The
clipsepackage is correctly positioned in alphabetical order.
37-37: LGTM!The
gnumakepackage is correctly positioned in alphabetical order.
73-83: LGTM!The Linux-conditional package block is well-organized:
ffmpegand other packages are correctly positioned alphabetically- The previous review concern about
opencodeandpowertopordering is resolved since they're now in separate conditional blocks
84-93: Good separation of CI-unnecessary packages.The logic to exclude desktop applications and GUI tools from CI builds is sensible and will improve CI performance. The package list is correctly maintained in alphabetical order.
Note: This depends on the
isCIimplementation being fixed (see earlier comment about impurity).
| let | ||
| isCI = builtins.getEnv "CI" != ""; | ||
| in |
There was a problem hiding this comment.
Avoid impure builtins.getEnv for environment-dependent logic.
Using builtins.getEnv breaks Nix's reproducibility guarantees because it reads from the environment at evaluation time. This can cause:
- Different evaluation results in different environments
- Issues with Nix's binary cache and substitutes
- Unpredictable behavior when the same expression is evaluated in different contexts
Consider passing isCI as a function parameter instead:
-let
- isCI = builtins.getEnv "CI" != "";
-in
+{ pkgs, inputs, isCI ? false }:Then pass the value from the calling module based on your build context. This maintains purity while achieving the same goal.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In home-manager/packages/default.nix around lines 5 to 7, remove the impure
builtins.getEnv usage and instead make isCI a function argument (or attribute)
to the file/module; replace the let-bound isCI with an input parameter (e.g.
isCI ? false) and use that parameter throughout, then update any callers/imports
of this file to pass the CI value from their environment or configuration (so
evaluation stays pure) and remove any direct calls to builtins.getEnv in this
file.
Enhance package configurations by adding btop and adjusting default and optional packages. Fix Zsh settings to enable the program and update initialization content. Comment out unnecessary integration settings in direnv.
Summary by cubic
Add btop as a managed Home Manager program, enable Zsh and direnv integrations, and streamline packages by moving desktop utilities to Linux-only optional packages and excluding them in CI. This reduces default bloat and improves shell and CI reliability.
New Features
Refactors
Written for commit d962580. Summary will update automatically on new commits.