refactor git - #312
Conversation
…tings - Moved user settings (name and email) under a new 'user' section. - Consolidated alias settings under a new 'alias' section. - Added 'enableGitIntegration' to delta configuration. - Cleaned up the overall structure for better readability.
- Updated the configuration key from 'extraConfig' to 'settings' to enhance clarity and maintain consistency with other configuration sections.
- Introduced Atuin configuration in home-manager. - Updated homebrew configuration to include Atuin desktop application. - Note: Atuin integration is temporarily disabled due to build issues.
- Introduced Delta configuration in home-manager with Git integration enabled. - Updated default.nix to include Delta in the program imports. - Removed temporary disablement note for Atuin configuration.
|
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
WalkthroughThis PR introduces new programs (atuin, delta) to home-manager, restructures git configuration to use nested settings instead of extraConfig, enables delta git integration, adds Linux-specific package conditionals, installs fzf-make, and adds atuin and related macOS packages to homebrew configuration. Changes
Sequence DiagramsequenceDiagram
participant Config as Git Config
participant Settings as Nested Settings
participant Delta as Delta Program
rect rgb(230, 245, 255)
Note over Config,Settings: Configuration Restructuring
Config->>Settings: User credentials: top-level → settings.user
Config->>Settings: Aliases: top-level → settings.alias
Config->>Settings: Git core options → settings.core (expanded)
Config->>Settings: GPG signing: extraConfig → settings
end
rect rgb(240, 250, 240)
Note over Delta: New Delta Integration
Settings->>Delta: enable = true
Settings->>Delta: enableGitIntegration = true
Settings->>Delta: navigate = true (new)
Settings->>Delta: dark = true (new)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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 focuses on enhancing the configuration management of development tools. It refactors existing Git configurations for better organization and consistency, introduces new declarative integrations for Atuin and Delta into the 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;DRRefactored Git configuration for clarity and consistency, integrated Delta and Atuin into home-manager, and added new Homebrew casks. What changed?
Description generated by Mesa. Update settings |
- Included fzf-make in the home-manager package imports. - Commented out yazi from the package list for potential future re-enablement.
There was a problem hiding this comment.
Code Review
This pull request refactors the Git configuration for better clarity, introduces support for Atuin and Delta, and adds several new applications to the Homebrew setup. The refactoring of the Git configuration is a good improvement, centralizing settings and renaming extraConfig to settings for consistency. The additions of Atuin and Delta are also valuable.
However, I've found a few issues that need addressing:
- A critical indentation error in the Git configuration that would prevent core settings from being applied correctly.
- Redundant configuration for Delta, which is defined in two separate files.
- The new Atuin module file contains only commented-out code, which can be confusing.
I've provided specific suggestions to address these points. Once these are fixed, the pull request will be in great shape.
| editor = "nvim"; | ||
| compression = -1; | ||
| autocrlf = "input"; |
There was a problem hiding this comment.
These settings are intended to be inside the core attribute set, but they are incorrectly indented. This makes them siblings of core instead of children, which will cause the configuration to be applied incorrectly. Please indent these lines correctly.
editor = "nvim";
compression = -1;
autocrlf = "input";
| delta = { | ||
| enable = true; | ||
| enableGitIntegration = true; | ||
| }; |
| # programs.atuin = { | ||
| # enable = true; | ||
| # enableBashIntegration = true; | ||
| # enableFishIntegration = true; | ||
| # enableZshIntegration = true; | ||
| # }; |
There was a problem hiding this comment.
There was a problem hiding this comment.
Pull Request Overview
This PR refactors git and delta configuration in home-manager, extracts delta into a separate module, adds new homebrew casks, and updates git configuration to use the newer settings attribute instead of deprecated options.
- Refactored git configuration to use
settingsattribute for user info, aliases, and other git config options - Extracted delta configuration into a dedicated module in
home-manager/programs/delta/ - Added atuin module stub (currently commented out) and three new homebrew casks
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| nix-darwin/config/homebrew.nix | Added atuin-desktop, beekeeper-studio, and wezterm casks |
| named-hosts/galactica/default.nix | Updated git config from extraConfig to settings attribute |
| home-manager/programs/git/default.nix | Refactored git configuration to use settings structure and moved delta to separate module |
| home-manager/programs/delta/default.nix | Created new dedicated delta configuration module |
| home-manager/programs/default.nix | Added imports for atuin and delta modules |
| home-manager/programs/atuin/default.nix | Created atuin module stub (currently all commented out) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| delta = { | ||
| enable = true; | ||
| enableGitIntegration = true; | ||
| }; |
There was a problem hiding this comment.
Delta is now configured in both git/default.nix (lines 82-85) and the new delta/default.nix file. This duplicate configuration could lead to conflicts or unexpected behavior. Since delta has been extracted to its own module in delta/default.nix, the configuration here should be removed.
| delta = { | |
| enable = true; | |
| enableGitIntegration = true; | |
| }; |
| unshallow = "fetch --prune --tags --unshallow"; | ||
| }; | ||
| core = { | ||
| editor = "nvim"; |
There was a problem hiding this comment.
Inconsistent indentation: this line should be indented with 10 spaces (or 5 levels of 2 spaces) to align with other settings at the same level. Currently it has only 8 spaces.
| # programs.atuin = { | ||
| # enable = true; | ||
| # enableBashIntegration = true; | ||
| # enableFishIntegration = true; | ||
| # enableZshIntegration = true; | ||
| # }; |
There was a problem hiding this comment.
The entire atuin configuration is commented out, but the module is being imported and included in the programs list in default.nix. This creates an empty module that provides no functionality. Either enable the configuration or remove the module from the imports list until it's ready to be used.
| # programs.atuin = { | |
| # enable = true; | |
| # enableBashIntegration = true; | |
| # enableFishIntegration = true; | |
| # enableZshIntegration = true; | |
| # }; | |
| programs.atuin = { | |
| enable = true; | |
| enableBashIntegration = true; | |
| enableFishIntegration = true; | |
| enableZshIntegration = true; | |
| }; |
There was a problem hiding this comment.
Performed full review of 12d784f...9bc6893
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
6 files reviewed | 0 comments | Edit Agent Settings • Read Docs
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".
| settings = { | ||
| user = { | ||
| name = "Shun Kakinoki"; | ||
| email = "shunkakinoki@gmail.com"; | ||
| }; |
There was a problem hiding this comment.
Use of nonexistent programs.git.settings breaks Git config
The Git module now writes user, alias, and other options under settings, but Home Manager exposes those attributes as userName, userEmail, aliases, and extraConfig. There is no programs.git.settings option, so evaluating this module will fail with an unknown-option error and none of the Git configuration will be applied. Keep these values under the supported options to avoid breaking configuration.
Useful? React with 👍 / 👎.
| settings = { | ||
| commit.gpgSign = true; | ||
| tag.gpgSign = true; | ||
| }; |
There was a problem hiding this comment.
Renaming extraConfig to settings for Git in galactica host is invalid
The galactica host definition now sets Git signing options under settings, but the Home Manager programs.git module does not define such an option. Using settings will trigger an evaluation error (programs.git.settings does not exist) and drop the signing configuration. The original extraConfig attribute must be kept for these values to take effect.
Useful? React with 👍 / 👎.
- Included Atuin in the homebrew configuration to enhance package availability.
There was a problem hiding this comment.
2 issues found across 6 files
Prompt for AI agents (all 2 issues)
Understand the root cause of the following 2 issues and fix them.
<file name="home-manager/programs/atuin/default.nix">
<violation number="1" location="home-manager/programs/atuin/default.nix:2">
With every programs.atuin option commented out, this module returns an empty attrset, so Atuin never gets enabled even though this file was added to configure it. Please uncomment or add the actual configuration so the feature is applied.</violation>
</file>
<file name="named-hosts/galactica/default.nix">
<violation number="1" location="named-hosts/galactica/default.nix:43">
`programs.git` does not expose a `settings` attribute—Home Manager expects `extraConfig`, so this rename causes the GPG signing options to be dropped.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| @@ -0,0 +1,8 @@ | |||
| { | |||
| # programs.atuin = { | |||
There was a problem hiding this comment.
With every programs.atuin option commented out, this module returns an empty attrset, so Atuin never gets enabled even though this file was added to configure it. Please uncomment or add the actual configuration so the feature is applied.
Prompt for AI agents
Address the following comment on home-manager/programs/atuin/default.nix at line 2:
<comment>With every programs.atuin option commented out, this module returns an empty attrset, so Atuin never gets enabled even though this file was added to configure it. Please uncomment or add the actual configuration so the feature is applied.</comment>
<file context>
@@ -0,0 +1,8 @@
+{
+ # programs.atuin = {
+ # enable = true;
+ # enableBashIntegration = true;
</file context>
✅ Addressed in 9e8f2a2
| key = "shunkakinoki@gmail.com"; | ||
| }; | ||
| extraConfig = { | ||
| settings = { |
There was a problem hiding this comment.
programs.git does not expose a settings attribute—Home Manager expects extraConfig, so this rename causes the GPG signing options to be dropped.
Prompt for AI agents
Address the following comment on named-hosts/galactica/default.nix at line 43:
<comment>`programs.git` does not expose a `settings` attribute—Home Manager expects `extraConfig`, so this rename causes the GPG signing options to be dropped.</comment>
<file context>
@@ -40,7 +40,7 @@ inputs.nix-darwin.lib.darwinSystem {
key = "shunkakinoki@gmail.com";
};
- extraConfig = {
+ settings = {
commit.gpgSign = true;
tag.gpgSign = true;
</file context>
| settings = { | |
| extraConfig = { |
- Updated the nixpkgs lock file with the latest lastModified timestamp, narHash, and revision for improved dependency management.
- Reformatted the Git configuration in default.nix for improved readability and consistency. - No functional changes were made to the configuration settings.
- Temporarily commented out the fzf plugin configuration in default.nix for clarity and potential future re-enablement. - No functional changes were made to other plugins.
- Uncommented the Atuin program configuration to enable Bash, Fish, and Zsh integrations. - This change restores the functionality that was previously commented out for clarity.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 9 out of 10 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "atuin" | ||
| "bun" | ||
| "claude-squad" | ||
| "cmake" |
There was a problem hiding this comment.
The atuin package is being installed via Homebrew here, but it's also configured in home-manager/programs/atuin/default.nix with enable = true, which typically installs the package through Nix. This creates a potential conflict where the same tool would be installed from two different sources. Consider removing the Homebrew installation if the home-manager module is managing atuin.
| "atuin" | |
| "bun" | |
| "claude-squad" | |
| "cmake" | |
| "bun" | |
| "claude-squad" | |
| "cmake" |
- Added a line to source the _hm_load_env_file.fish function in the interactiveShellInit section of default.nix. - This change ensures that the environment variables are loaded correctly when starting an interactive Fish shell.
Note
Refactors Git to use
settingsand separate Delta config, adds Delta module and Atuin scaffold to home-manager, and adds new Homebrew casks.programs.git.settings(moves user/alias/core/etc.), adds global ignores; keeps LFS; enables Delta via separate module.programs/deltamodule enabling Delta with Git integration.programs/atuinscaffold and includes it in the program list.atuinanddeltainhome-manager/programs/default.nix.extraConfig→settingsfor GPG signing.atuin-desktop,beekeeper-studio,wezterm.Written by Cursor Bugbot for commit 9bc6893. Configure here.
Summary by cubic
Refactors Git config for clarity and consistency, enables Delta integration, and turns on Atuin with shell integrations in home-manager. Also updates Homebrew casks.
Refactors
New Features
Written for commit 0bcd3ff. Summary will update automatically on new commits.