Skip to content

Improve dotfiles robustness and remove legacy dependencies - #2

Merged
kincaidoneil merged 4 commits into
mainfrom
claude/review-dotfiles-mac-011CUecvQWFu6abkiupKDgMy
Oct 31, 2025
Merged

Improve dotfiles robustness and remove legacy dependencies#2
kincaidoneil merged 4 commits into
mainfrom
claude/review-dotfiles-mac-011CUecvQWFu6abkiupKDgMy

Conversation

@kincaidoneil

Copy link
Copy Markdown
Owner
  • Add sudo checks, error handling, update deprecated URLs, and improve robustness of install script
  • Remove unused GPG dependencies (using 1Password SSH agent now)
  • Add GitHub Actions workflow to test install on macOS and Linux
  • Simplify README

Install script improvements:
- Add sudo checks: prevent running as root, verify sudo access upfront
- Fix unsafe symlinks: add -f flag and proper quoting
- Remove Codespaces logic: simplify script structure
- Fix Docker security: use $USER instead of hardcoded username
- Update deprecated URLs: replace git.io shortlinks with full URLs
- Fix dotfiles clone: add explicit target directory and proper if block
- Fix Mac portability: replace readlink -f with portable alternative
- Move brew shellenv earlier: ensure brew command is available
- Remove redundant brew update: not needed on fresh install
- Add Homebrew error handling: clear error message on failure

Dependency cleanup:
- Remove GPG dependencies (gpg2, pinentry-mac) - using 1Password SSH agent
- Remove pinentry-mac keychain configuration
- Fix pnpm conflict: use corepack instead of direct npm install
- Simplify ngrok installation: ngrok/ngrok/ngrok -> ngrok
- Update zi install script to use get.zshell.dev with main branch

Config formatting:
- Fix inconsistent tabs/spacing in .gitconfig files (standardize to tabs)
- Collapse GPG/SSH setup into <details> section (legacy)
- Promote 1Password as recommended authentication method
- Fix add-user.sh command to use piping instead of source
- Add clear install instructions with feature list
- Remove verbose "What's Included" section
- Improve overall structure and readability
@kincaidoneil
kincaidoneil requested a review from Copilot October 31, 2025 16:46

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors and modernizes the dotfiles installation script and documentation. It removes the GitHub Codespaces conditional logic, improves error handling, and makes the installation process more portable across different user environments.

  • Removed hardcoded username references and Codespaces-specific logic
  • Added root user checks and improved error handling with explicit failure messages
  • Updated package installations and removed deprecated tools (gpg2/pinentry-mac on macOS)

Reviewed Changes

Copilot reviewed 4 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
install.sh Major refactor: added root/sudo checks, removed Codespaces logic, improved error handling, updated URLs and package lists
.zshrc Replaced hardcoded user paths with $HOME, removed pnpm manual PATH setup, fixed typo, improved Docker cleanup function
README.md Reorganized documentation with clearer structure, moved legacy GPG/SSH instructions to collapsible section
.github/workflows/test.yml Added CI workflow to test installation on Ubuntu and macOS
.gitconfig-linux Normalized indentation (spaces to tabs)
.gitconfig-darwin Normalized indentation (spaces to tabs), removed deprecated gpg2 references

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread install.sh Outdated
# Install LTS and latest versions of Node (-y accepts confirm prompt, -n prevents modifying .zshrc, which already references n)
curl -L https://git.io/n-install | bash -s -- -y -n lts latest
# Fix Docker permissions issue: https://superuser.com/questions/835696/how-solve-permission-problems-for-docker-in-ubuntu
sudo gpasswd -a $USER docker

Copilot AI Oct 31, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The variable $USER should be quoted to prevent word splitting and glob expansion. Change to \"$USER\".

Suggested change
sudo gpasswd -a $USER docker
sudo gpasswd -a "$USER" docker

Copilot uses AI. Check for mistakes.
Comment thread install.sh

curl -fsSL https://bun.sh/install | bash
# Set default shell to ZSH on Linux
sudo chsh -s /bin/zsh $USER

Copilot AI Oct 31, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The variable $USER should be quoted to prevent word splitting and glob expansion. Change to \"$USER\".

Copilot uses AI. Check for mistakes.
Comment thread install.sh
git \
make \
mkcert `# Tool to sign local certs for development` \
ngrok \

Copilot AI Oct 31, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The package name has changed from ngrok/ngrok/ngrok (tap-based) to just ngrok. This may cause installation to fail if the formula doesn't exist under this name. Verify that brew install ngrok works correctly, or revert to the tap-based installation.

Copilot uses AI. Check for mistakes.
Comment thread .zshrc
docker stop $(docker ps -q)
docker rm $(docker ps -a -q)
fi
docker stop $(docker ps -q) 2>/dev/null || true

Copilot AI Oct 31, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there are no running containers, docker ps -q returns empty output, causing docker stop to be called with no arguments, which may produce an error message despite the || true. Consider checking if containers exist first: [ -n \"$(docker ps -q)\" ] && docker stop $(docker ps -q) before running the system prune.

Suggested change
docker stop $(docker ps -q) 2>/dev/null || true
[ -n "$(docker ps -q)" ] && docker stop $(docker ps -q) 2>/dev/null || true

Copilot uses AI. Check for mistakes.
- Fix hardcoded username: /Users/kincaid -> $HOME
- Remove pnpm PATH section (not needed with corepack for local deps)
- Consolidate duplicate Homebrew checks into single if block
- Improve docker-clean: use docker system prune for comprehensive cleanup
- Fix typo: initializaiton -> initialization
@kincaidoneil
kincaidoneil force-pushed the claude/review-dotfiles-mac-011CUecvQWFu6abkiupKDgMy branch from 703d75a to f3ae4d0 Compare October 31, 2025 17:17
- Test on both ubuntu-latest and macos-latest
- Remove pre-installed Node on macOS (conflicts with n-install)
- Skip apt dist-upgrade on Linux in CI (slow kernel updates)
- Skip Docker installation on Linux in CI (package conflicts)
- Use zsh shell for verification to auto-source .zshrc
- Verify dotfiles are symlinked and key commands exist
@kincaidoneil
kincaidoneil force-pushed the claude/review-dotfiles-mac-011CUecvQWFu6abkiupKDgMy branch from f3ae4d0 to 200e406 Compare October 31, 2025 17:19
@kincaidoneil
kincaidoneil merged commit a4f891f into main Oct 31, 2025
2 checks passed
@kincaidoneil
kincaidoneil deleted the claude/review-dotfiles-mac-011CUecvQWFu6abkiupKDgMy branch October 31, 2025 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants