Skip to content

Conversation

@klei22
Copy link
Collaborator

@klei22 klei22 commented Oct 20, 2025

This pull request improves the robustness and reliability of the development environment setup scripts. The main changes include enhanced error handling, safer environment variable usage, and improved checks to prevent duplicate environment creation and shell configuration entries.

Script robustness and error handling:

  • Added #!/bin/bash and set -euo pipefail to 00-setup-conda.sh and install_all.sh to ensure scripts exit on errors and undefined variables, making them safer to run on new machines. [1] [2]

Conda environment setup improvements:

  • Refactored 00-setup-conda.sh to use variables for paths and environment names, check for existing Conda environments before creating, and avoid duplicate conda activate lines in .zshrc and .bashrc.
  • Updated install_all.sh to explicitly source the Conda initialization script and activate the reallmforge environment, with error handling if the script is missing.

Package installation:

  • Improved system package installation in 00-setup-conda.sh by running sudo apt update before installing packages and adding the -y flag for non-interactive installs.

@klei22 klei22 requested review from Copilot and gkielian October 20, 2025 18:31
Copy link

Copilot AI left a comment

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 enhances the development environment setup scripts by adding robust error handling, preventing duplicate configurations, and improving package installation reliability. The changes make the scripts safer for first-time setup on new machines while preventing issues from multiple executions.

Key changes:

  • Added strict error handling with set -euo pipefail to both setup scripts
  • Implemented checks to prevent duplicate Conda environment creation and shell configuration entries
  • Added explicit Conda environment activation with error handling in the main installation script

Reviewed Changes

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

File Description
dev_env_setup_scripts/install_all.sh Added pipefail to error handling and explicit Conda environment activation with validation
dev_env_setup_scripts/00-setup-conda.sh Refactored to use variables, added duplicate prevention checks, and improved package installation

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

conda create --name reallmforge python=3.10
echo 'conda activate reallmforge' >> ~/.zshrc
echo 'conda activate reallmforge' >> ~/.bashrc
if ! conda info --envs | awk '{print $1}' | grep -qx "$ENV_NAME"; then
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

The awk command extracts the first column, but conda info --envs output includes comment lines (starting with #) and a header line. These will be included in the grep search, potentially causing false matches. Use grep -v '^#' to filter out comment lines before the awk command: if ! conda info --envs | grep -v '^#' | awk '{print $1}' | grep -qx \"$ENV_NAME\"; then

Suggested change
if ! conda info --envs | awk '{print $1}' | grep -qx "$ENV_NAME"; then
if ! conda info --envs | grep -v '^#' | awk '{print $1}' | grep -qx "$ENV_NAME"; then

Copilot uses AI. Check for mistakes.
Comment on lines +25 to +26
log "Activating conda environment..."
CONDA_SH="$HOME/miniconda3/etc/profile.d/conda.sh"
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

The hardcoded path $HOME/miniconda3 duplicates the installation directory defined in 00-setup-conda.sh as MINICONDA_DIR. If the installation path changes in one script, it must be updated here as well. Consider extracting this to a shared configuration variable or sourcing it from the previous script.

Suggested change
log "Activating conda environment..."
CONDA_SH="$HOME/miniconda3/etc/profile.d/conda.sh"
# Source MINICONDA_DIR from 00-setup-conda.sh (assumes it exports MINICONDA_DIR)
source ./00-setup-conda.sh
CONDA_SH="$MINICONDA_DIR/etc/profile.d/conda.sh"

Copilot uses AI. Check for mistakes.
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.

1 participant