feat: rewrite as Go CLI with containers and - #1
Conversation
comprehensive docs Replace shell scripts with Go CLI tool, add Docker/K8s support, create beginner-friendly documentation, and remove vendored dependencies.
Summary of ChangesHello @thenameiswiiwin, 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 marks a pivotal transformation of the project, evolving from an ad-hoc collection of shell scripts into a robust, Go-powered development environment manager. The core functionality is now encapsulated within a new CLI tool, providing a unified interface for setting up diverse language-specific development environments, integrating containerization for consistency, and offering initial support for Kubernetes deployments. This extensive overhaul, coupled with a wealth of new documentation, aims to enhance the project's usability, scalability, and long-term maintainability. 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. 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
|
![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 is an impressive and comprehensive rewrite. The move from disparate shell scripts to a structured Go CLI with a data-driven manifest approach is a massive improvement for maintainability, consistency, and security. The addition of container support, devcontainer configurations, and Kubernetes manifests makes the project much more versatile and modern. The documentation is outstanding, with guides for all levels of users.
My review focuses on a few areas to further improve security and maintainability, mainly around adhering to the project's own excellent security guidelines (avoiding curl | bash, pinning versions) and making some parts of the new CLI even more data-driven and robust.
Overall, this is a fantastic pull request that transforms the project for the better.
| cmd := exec.Command("bash", "-c", | ||
| `/bin/bash -c "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/HEAD/install.sh)"`) |
There was a problem hiding this comment.
This command pipes curl directly to bash, which is a security risk and violates the project's own guideline in AGENTS.md ("no curl | sudo without checksum/signature"). Although Homebrew's installer is generally trusted, it's better to adhere to the security policy. Consider downloading the script to a temporary file, verifying its checksum, and then executing it.
| func installCoreDependencies() { | ||
| coreTools := []string{"git", "curl", "zsh", "tmux", "ripgrep", "fd", "fzf"} | ||
|
|
||
| log.Info("\nInstalling core dependencies...") | ||
|
|
||
| for _, tool := range coreTools { | ||
| if commandExists(tool) { | ||
| log.Debug("%s already installed", tool) | ||
| continue | ||
| } | ||
|
|
||
| log.Action("Installing %s", tool) | ||
|
|
||
| if dryRun { | ||
| continue | ||
| } | ||
|
|
||
| var cmd *exec.Cmd | ||
| switch osInfo.PackageManager { | ||
| case "brew": | ||
| cmd = exec.Command("brew", "install", tool) | ||
| case "apt": | ||
| cmd = exec.Command("sudo", "apt-get", "install", "-y", tool) | ||
| case "pacman": | ||
| cmd = exec.Command("sudo", "pacman", "-S", "--noconfirm", tool) | ||
| case "yay": | ||
| cmd = exec.Command("yay", "-S", "--noconfirm", tool) | ||
| default: | ||
| log.Error("Unsupported package manager: %s", osInfo.PackageManager) | ||
| continue | ||
| } | ||
|
|
||
| if verbose { | ||
| cmd.Stdout = os.Stdout | ||
| cmd.Stderr = os.Stderr | ||
| } | ||
|
|
||
| if err := cmd.Run(); err != nil { | ||
| log.Warn("Failed to install %s: %v", tool, err) | ||
| } else { | ||
| log.Success("Installed %s", tool) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The installCoreDependencies function uses a hardcoded list of tools. This is inconsistent with the data-driven approach used for presets and can lead to maintainability issues (e.g., dev doctor checks for nvim, which is not installed here).
Consider refactoring this to load manifests/base.yaml and use the installer package to install the base packages, similar to how preset apply works. This would centralize all package definitions in manifests, improve consistency, and make it easier to manage the core dependencies.
| RUN curl -LO https://github.com/neovim/neovim/releases/latest/download/nvim-linux64.tar.gz \ | ||
| && tar -xzf nvim-linux64.tar.gz \ | ||
| && mv nvim-linux64/bin/nvim /usr/local/bin/ \ | ||
| && mv nvim-linux64/share/nvim /usr/local/share/ \ | ||
| && mv nvim-linux64/lib/nvim /usr/local/lib/ \ | ||
| && rm -rf nvim-linux64 nvim-linux64.tar.gz |
There was a problem hiding this comment.
Downloading from a /latest/ URL makes the build non-reproducible and can introduce unexpected changes. This violates the version pinning guidelines in AGENTS.md and docs/VERSION_PINNING.md.
Please pin to a specific version of Neovim and verify its checksum. For example:
ARG NVIM_VERSION=v0.10.0
ARG NVIM_SHA256=...
RUN curl -LO https://github.com/neovim/neovim/releases/download/${NVIM_VERSION}/nvim-linux64.tar.gz \
&& echo "${NVIM_SHA256} nvim-linux64.tar.gz" | sha256sum -c - \
&& ...This comment also applies to the other Dockerfiles (python/Dockerfile, rust/Dockerfile, web/Dockerfile).
| go install golang.org/x/tools/gopls@latest | ||
|
|
||
| # Install golangci-lint | ||
| RUN curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b /usr/local/bin |
There was a problem hiding this comment.
This command uses curl | sh to install golangci-lint from the master branch, which is insecure and not reproducible. This violates the project's security guidelines.
Please download a specific versioned binary from the golangci-lint releases page, verify its checksum, and then place it in /usr/local/bin.
|
|
||
| # Install pnpm and bun | ||
| RUN npm install -g pnpm && \ | ||
| curl -fsSL https://bun.sh/install | bash |
There was a problem hiding this comment.
This command uses curl | bash to install bun. This is a security risk and not ideal for reproducible builds. Please consider downloading a specific version of the bun binary from its releases, verifying its checksum, and installing it manually.
| @command -v shellcheck >/dev/null 2>&1 || { echo "shellcheck not installed. Install with: brew install shellcheck"; exit 1; } | ||
| @command -v shfmt >/dev/null 2>&1 || { echo "shfmt not installed. Install with: brew install shfmt"; exit 1; } | ||
| @echo "Running shellcheck..." | ||
| @find . -name "*.sh" -o -path "./runs/*" -type f -executable | while read -r file; do \ |
There was a problem hiding this comment.
The path ./runs/* seems to be a leftover from the old project structure, as the runs/ directory has been removed in this refactoring. This part of the find command can be removed to avoid confusion and keep the Makefile clean.
@find . -name "*.sh" -type f -executable | while read -r file; do \
| case "rust": | ||
| checkCmd = "rustc --version && cargo --version" | ||
| case "web": | ||
| checkCmd = "node --version && npm --version" |
There was a problem hiding this comment.
The smoke test for the web preset could be more comprehensive. The preset installs pnpm and bun in addition to node and npm, but the test only verifies the latter two. Consider adding checks for pnpm and bun to ensure all key components of the preset are correctly installed in the container.
| checkCmd = "node --version && npm --version" | |
| checkCmd = "node --version && npm --version && pnpm --version && bun --version" |
| log.Info("Available presets:") | ||
| log.Info(" • python - Python development (uv/pyenv, pyright, ruff, pytest)") | ||
| log.Info(" • go - Go development (gopls, gofumpt, golangci-lint, delve)") | ||
| log.Info(" • rust - Rust development (rustup, rust-analyzer, clippy)") | ||
| log.Info(" • web - Web development (Node.js, TypeScript, React, Tailwind)") | ||
| log.Info("\nUsage: dev preset apply <preset>") | ||
| }, |
There was a problem hiding this comment.
The list of available presets is hardcoded. This creates a maintenance burden, as this list must be manually updated whenever a new preset manifest is added to manifests/presets/. It would be more robust to generate this list dynamically by reading the filenames from the manifests/presets/ directory.
| func fileExists(path string) bool { | ||
| cmd := exec.Command("test", "-f", path) | ||
| return cmd.Run() == nil | ||
| } |
There was a problem hiding this comment.
The fileExists function shells out to the test command. It's more idiomatic and robust in Go to use the os package for file system operations. Using os.Stat would provide better error handling and remove the dependency on an external command.
| func fileExists(path string) bool { | |
| cmd := exec.Command("test", "-f", path) | |
| return cmd.Run() == nil | |
| } | |
| func fileExists(path string) bool { | |
| _, err := os.Stat(path) | |
| return err == nil | |
| } |
| func readFile(path string) string { | ||
| cmd := exec.Command("cat", path) | ||
| output, err := cmd.Output() | ||
| if err != nil { | ||
| return "" | ||
| } | ||
| return string(output) | ||
| } |
There was a problem hiding this comment.
The readFile function shells out to the cat command. This can be replaced with os.ReadFile for a more robust, portable, and idiomatic Go implementation. It also provides better error handling.
func readFile(path string) string {
content, err := os.ReadFile(path)
if err != nil {
return ""
}
return string(content)
}1 participant
Replace shell scripts with Go CLI tool, add Docker/K8s
support, create beginner-friendly documentation, and
remove vendored dependencies.