feat: add npm, ssh, kitty, and kubernetes dotfile support#89
Conversation
|
Claude finished @AnnatarHe's task —— View job PR Review Complete ✅Reviewing the dotfile support additions for npm, ssh, kitty, and kubernetes. Todo List:
Code Review Feedback✅ Strengths
|
There was a problem hiding this comment.
Summary of Changes
Hello @AnnatarHe, 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 significantly expands the shelltime tool's capabilities by integrating support for managing configuration files for four additional applications: npm, SSH, Kitty, and Kubernetes. This allows users to easily push and pull their critical dotfiles for these tools, enhancing the tool's utility for system configuration management.
Highlights
- New Dotfile Support: Introduced support for managing dotfiles for npm, SSH, Kitty terminal, and Kubernetes applications.
- Application Implementations: Created dedicated
NpmApp,SshApp,KittyApp, andKubernetesAppstructs with their respective methods to define their dotfile locations and collection logic. - Command Integration: Updated
dotfiles_pull.goanddotfiles_push.goto register and include the newly added dotfile applications, making them available forpullandpushoperations.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.
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
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
Codecov Report❌ Patch coverage is
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 2 files with indirect coverage changes 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Code Review
This pull request adds support for npm, ssh, kitty, and kubernetes dotfiles. The implementation correctly adds the new applications. However, I've identified two areas for improvement regarding code duplication and maintainability. First, the list of supported applications is duplicated in two files, which could be centralized. Second, the new application handlers introduce significant boilerplate code. I've suggested a more generic approach to reduce this duplication and make it easier to add new simple applications in the future. Addressing these points would improve the long-term maintainability of the codebase.
| package model | ||
|
|
||
| import "context" | ||
|
|
||
| // KittyApp handles Kitty terminal configuration files | ||
| type KittyApp struct { | ||
| BaseApp | ||
| } | ||
|
|
||
| func NewKittyApp() DotfileApp { | ||
| return &KittyApp{} | ||
| } | ||
|
|
||
| func (k *KittyApp) Name() string { | ||
| return "kitty" | ||
| } | ||
|
|
||
| func (k *KittyApp) GetConfigPaths() []string { | ||
| return []string{ | ||
| "~/.config/kitty/", | ||
| } | ||
| } | ||
|
|
||
| func (k *KittyApp) CollectDotfiles(ctx context.Context) ([]DotfileItem, error) { | ||
| return k.CollectFromPaths(ctx, k.Name(), k.GetConfigPaths()) | ||
| } |
There was a problem hiding this comment.
This file, along with dotfile_kubernetes.go, dotfile_npm.go, and dotfile_ssh.go, contains a lot of boilerplate code. All these new application implementations are almost identical, only differing in the application name and configuration paths. This pattern will make it cumbersome to add more simple applications in the future.
To improve maintainability and reduce code duplication, I suggest creating a generic DotfileApp implementation that can be configured with a name and paths. For example:
// In model/dotfile_apps.go or a new file
type ConfigurableApp struct {
BaseApp
appName string
configPaths []string
}
func NewConfigurableApp(name string, paths []string) DotfileApp {
return &ConfigurableApp{
appName: name,
configPaths: paths,
}
}
func (a *ConfigurableApp) Name() string {
return a.appName
}
func (a *ConfigurableApp) GetConfigPaths() []string {
return a.configPaths
}
func (a *ConfigurableApp) CollectDotfiles(ctx context.Context) ([]DotfileItem, error) {
return a.CollectFromPaths(ctx, a.Name(), a.GetConfigPaths())
}With this, you could remove the new dotfile_*.go files and instantiate the apps directly where they are needed, for example in commands/dotfiles_pull.go:
// ...
"kitty": model.NewConfigurableApp("kitty", []string{"~/.config/kitty/"}),
"kubernetes": model.NewConfigurableApp("kubernetes", []string{"~/.kube/config"}),
// ...This would significantly simplify adding new applications that follow this simple file/directory pattern.
| allApps := map[string]model.DotfileApp{ | ||
| "nvim": model.NewNvimApp(), | ||
| "fish": model.NewFishApp(), | ||
| "git": model.NewGitApp(), | ||
| "zsh": model.NewZshApp(), | ||
| "bash": model.NewBashApp(), | ||
| "ghostty": model.NewGhosttyApp(), | ||
| "claude": model.NewClaudeApp(), | ||
| "nvim": model.NewNvimApp(), | ||
| "fish": model.NewFishApp(), | ||
| "git": model.NewGitApp(), | ||
| "zsh": model.NewZshApp(), | ||
| "bash": model.NewBashApp(), | ||
| "ghostty": model.NewGhosttyApp(), | ||
| "claude": model.NewClaudeApp(), | ||
| "npm": model.NewNpmApp(), | ||
| "ssh": model.NewSshApp(), | ||
| "kitty": model.NewKittyApp(), | ||
| "kubernetes": model.NewKubernetesApp(), | ||
| } |
There was a problem hiding this comment.
There's a duplication of the list of available applications here and in commands/dotfiles_push.go. This means that every time a new application is supported, it needs to be added in two different places, which is error-prone and increases maintenance overhead.
Consider centralizing the application list. You could create a function, for example model.GetAllApps(), that returns a slice of all DotfileApp implementations.
Then, in dotfiles_push.go, you would just call this function. Here in dotfiles_pull.go, you could iterate over the slice to build the map:
allApps := make(map[string]model.DotfileApp)
for _, app := range model.GetAllApps() {
allApps[app.Name()] = app
}This would make adding new apps simpler and safer.
|
rebase and solve the conflicts @claude |
|
Claude finished @AnnatarHe's task —— View job Rebase and Conflict Resolution Complete ✅I've successfully rebased the branch and resolved all conflicts! The branch is now clean and ready. Todo List
What Was DoneProblem Identified: The original branch contained 29 commits with conflicts due to older infrastructure changes that had since been merged into main through other PRs. Solution Applied:
Result:
The PR is now ready for review and merge! The branch |
- Add NpmApp for ~/.npmrc configuration - Add SshApp for ~/.ssh/config configuration - Add KittyApp for ~/.config/kitty/ directory - Add KubernetesApp for ~/.kube/config configuration - Update dotfiles_pull.go and dotfiles_push.go to include all new apps Co-authored-by: Le He <AnnatarHe@users.noreply.github.com>
e3162e7 to
3bc5cb0
Compare
Adds support for 4 new dotfile applications: npm, ssh, kitty, and kubernetes.
Changes
~/.npmrcconfiguration~/.ssh/configconfiguration~/.config/kitty/directory~/.kube/configconfigurationUsage
Fixes #87
🤖 Generated with Claude Code