-
-
Notifications
You must be signed in to change notification settings - Fork 103
Update Conductor config to support multiple version managers #867
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
0fe455f
5513e24
19d10af
454e037
d555c63
13c606f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| #!/usr/bin/env zsh | ||
| # Wrapper script to run commands in Conductor with proper tool versions | ||
| # Usage: bin/conductor-exec <command> [args...] | ||
| # | ||
| # This is needed because Conductor (and other non-interactive shells) don't | ||
| # source .zshrc, so version manager PATH configuration isn't active by default. | ||
| # | ||
| # - If mise is available: uses `mise exec` for correct tool versions | ||
| # - Otherwise: falls back to direct execution (for asdf/rbenv/nvm users) | ||
| # | ||
| # Examples: | ||
| # bin/conductor-exec ruby --version # Uses correct Ruby version | ||
| # bin/conductor-exec bundle exec rubocop # Correct Ruby for linting | ||
| # bin/conductor-exec git commit -m "msg" # Pre-commit hooks work correctly | ||
| # bin/conductor-exec yarn install # Uses correct Node version | ||
| # | ||
| # See: https://github.com/shakacode/react_on_rails-demos/issues/105 | ||
|
|
||
| if command -v mise &> /dev/null; then | ||
| exec mise exec -- "$@" | ||
| else | ||
| # Fall back to direct execution for non-mise users | ||
| exec "$@" | ||
| fi | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,43 +1,94 @@ | ||
| #!/usr/bin/env bash | ||
| set -euo pipefail | ||
| #!/bin/zsh | ||
| set -e | ||
|
||
|
|
||
| echo "🔧 Setting up Shakapacker workspace..." | ||
|
|
||
| # Set up Ruby version if asdf is available | ||
| if command -v asdf &> /dev/null; then | ||
| echo "📝 Using asdf Ruby version management..." | ||
| # Ensure we have the right Ruby version file | ||
| echo "ruby 3.3.4" > .tool-versions | ||
| # Use asdf exec to run commands with the right Ruby version | ||
| BUNDLE_CMD="asdf exec bundle" | ||
| # Detect and initialize version manager | ||
| # Supports: mise, asdf, or direct PATH (rbenv/nvm/nodenv already in PATH) | ||
| VERSION_MANAGER="none" | ||
|
|
||
| echo "📋 Detecting version manager..." | ||
|
|
||
| if command -v mise &> /dev/null; then | ||
| VERSION_MANAGER="mise" | ||
| echo "✅ Found mise" | ||
| # Trust mise config for current directory only and install tools | ||
| mise trust 2>/dev/null || true | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: The Prompt To Fix With AIThis is a comment left during a code review.
Path: conductor-setup.sh
Line: 16:16
Comment:
**style:** The `mise trust` command trusts the mise configuration in the current directory. This is necessary for mise to work, but could be a security concern if running in untrusted repositories. The old script didn't require this trust step.
How can I resolve this? If you propose a fix, please make it concise. |
||
| mise install | ||
| elif [[ -f ~/.asdf/asdf.sh ]]; then | ||
| VERSION_MANAGER="asdf" | ||
| source ~/.asdf/asdf.sh | ||
| echo "✅ Found asdf (from ~/.asdf/asdf.sh)" | ||
| elif command -v asdf &> /dev/null; then | ||
| VERSION_MANAGER="asdf" | ||
| # For homebrew-installed asdf | ||
| if [[ -f /opt/homebrew/opt/asdf/libexec/asdf.sh ]]; then | ||
| source /opt/homebrew/opt/asdf/libexec/asdf.sh | ||
| fi | ||
| echo "✅ Found asdf" | ||
| else | ||
| BUNDLE_CMD="bundle" | ||
| echo "ℹ️ No version manager detected, using system PATH" | ||
| echo " (Assuming rbenv/nvm/nodenv or system tools are already configured)" | ||
| fi | ||
|
Comment on lines
12
to
31
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: The old script created Prompt To Fix With AIThis is a comment left during a code review.
Path: conductor-setup.sh
Line: 12:32
Comment:
**style:** The old script created `.tool-versions` file with `echo "ruby 3.3.4" > .tool-versions` to ensure asdf users had a Ruby version specified. The new script removes this, so asdf/mise users must have a pre-existing `.tool-versions` or `.mise.toml` file in the repository or their home directory. Check that all supported version managers will have access to appropriate version configuration.
How can I resolve this? If you propose a fix, please make it concise. |
||
|
|
||
| # Check for required tools | ||
| if ! $BUNDLE_CMD --version &> /dev/null; then | ||
| echo "❌ Error: Ruby bundler is not installed" | ||
| echo "Please install bundler first: gem install bundler" | ||
| # Helper function to run commands with the detected version manager | ||
| run_cmd() { | ||
| if [[ "$VERSION_MANAGER" == "mise" ]] && [[ -x "bin/conductor-exec" ]]; then | ||
| bin/conductor-exec "$@" | ||
| else | ||
| "$@" | ||
| fi | ||
| } | ||
|
|
||
| # Check required tools | ||
| echo "📋 Checking required tools..." | ||
| run_cmd ruby --version >/dev/null 2>&1 || { echo "❌ Error: Ruby is not installed or not in PATH."; exit 1; } | ||
| run_cmd node --version >/dev/null 2>&1 || { echo "❌ Error: Node.js is not installed or not in PATH."; exit 1; } | ||
|
|
||
| # Check Ruby version | ||
| RUBY_VERSION=$(run_cmd ruby -v | awk '{print $2}') | ||
| MIN_RUBY_VERSION="2.7.0" | ||
| if [[ $(echo -e "$MIN_RUBY_VERSION\n$RUBY_VERSION" | sort -V | head -n1) != "$MIN_RUBY_VERSION" ]]; then | ||
| echo "❌ Error: Ruby version $RUBY_VERSION is too old. Shakapacker requires Ruby >= 2.7.0" | ||
| echo " Please upgrade Ruby using your version manager or system package manager." | ||
| exit 1 | ||
| fi | ||
| echo "✅ Ruby version: $RUBY_VERSION" | ||
|
|
||
| if ! command -v yarn &> /dev/null; then | ||
| echo "❌ Error: Yarn is not installed" | ||
| echo "Please install yarn first" | ||
| # Check Node version | ||
| NODE_VERSION=$(run_cmd node -v | cut -d'v' -f2) | ||
| MIN_NODE_VERSION="14.0.0" | ||
| if [[ $(echo -e "$MIN_NODE_VERSION\n$NODE_VERSION" | sort -V | head -n1) != "$MIN_NODE_VERSION" ]]; then | ||
| echo "❌ Error: Node.js version v$NODE_VERSION is too old. Shakapacker requires Node.js >= 14.0.0" | ||
| echo " Please upgrade Node.js using your version manager or system package manager." | ||
| exit 1 | ||
| fi | ||
| echo "✅ Node.js version: v$NODE_VERSION" | ||
|
|
||
| # Copy any environment files from root if they exist | ||
| if [ -n "${CONDUCTOR_ROOT_PATH:-}" ]; then | ||
| if [ -f "$CONDUCTOR_ROOT_PATH/.env" ]; then | ||
| echo "📝 Copying .env file..." | ||
| cp "$CONDUCTOR_ROOT_PATH/.env" .env | ||
| fi | ||
|
|
||
| if [ -f "$CONDUCTOR_ROOT_PATH/.env.local" ]; then | ||
| echo "📝 Copying .env.local file..." | ||
| cp "$CONDUCTOR_ROOT_PATH/.env.local" .env.local | ||
| fi | ||
| fi | ||
|
|
||
| # Install Ruby dependencies | ||
| echo "📦 Installing Ruby dependencies..." | ||
| $BUNDLE_CMD install | ||
| echo "💎 Installing Ruby dependencies..." | ||
| run_cmd bundle install | ||
|
|
||
| # Install JavaScript dependencies | ||
| echo "📦 Installing JavaScript dependencies..." | ||
| yarn install --frozen-lockfile | ||
| run_cmd yarn install --frozen-lockfile | ||
|
|
||
| # Set up Husky git hooks | ||
| echo "🪝 Setting up Husky git hooks..." | ||
| npx husky | ||
| run_cmd npx husky | ||
| if [ ! -f .husky/pre-commit ]; then | ||
| echo "Creating pre-commit hook..." | ||
| cat > .husky/pre-commit << 'EOF' | ||
|
|
@@ -47,24 +98,20 @@ EOF | |
| chmod +x .husky/pre-commit | ||
| fi | ||
|
|
||
| # Copy environment files if they exist in root | ||
| if [ -n "${CONDUCTOR_ROOT_PATH:-}" ]; then | ||
| if [ -f "$CONDUCTOR_ROOT_PATH/.env" ]; then | ||
| echo "📋 Copying .env file from root..." | ||
| cp "$CONDUCTOR_ROOT_PATH/.env" .env | ||
| fi | ||
|
|
||
| if [ -f "$CONDUCTOR_ROOT_PATH/.env.local" ]; then | ||
| echo "📋 Copying .env.local file from root..." | ||
| cp "$CONDUCTOR_ROOT_PATH/.env.local" .env.local | ||
| fi | ||
| fi | ||
| # Run initial linting checks | ||
| echo "✅ Running initial linting checks..." | ||
| run_cmd bundle exec rubocop --version | ||
|
|
||
coderabbitai[bot] marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| echo "✅ Workspace setup complete!" | ||
| echo "✨ Workspace setup complete!" | ||
| echo "" | ||
| echo "📚 Key commands:" | ||
| echo " • bundle exec rspec - Run Ruby tests" | ||
| echo " • bundle exec rake run_spec:gem - Run gem-specific tests" | ||
| echo " • yarn test - Run JavaScript tests" | ||
| echo " • yarn lint - Run JavaScript linting" | ||
| echo " • bundle exec rubocop - Run Ruby linting (required before commits)" | ||
| echo "" | ||
| echo "Available commands:" | ||
| echo " - Run tests: bundle exec rspec" | ||
| echo " - Run specific test suites: bundle exec rake run_spec:gem" | ||
| echo " - Run JavaScript tests: yarn test" | ||
| echo " - Lint JavaScript: yarn lint" | ||
| echo " - Lint Ruby: bundle exec rubocop" | ||
| if [[ "$VERSION_MANAGER" == "mise" ]]; then | ||
| echo "💡 Tip: Use 'bin/conductor-exec <command>' if tool versions aren't detected correctly." | ||
| fi | ||
| echo "⚠️ Remember: Always run 'bundle exec rubocop' before committing!" | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,9 @@ | ||
| { | ||
| "scripts": { | ||
| "setup": "./conductor-setup.sh", | ||
| "run": "bundle exec rspec", | ||
| "run": "bin/conductor-exec bundle exec rspec", | ||
| "test": "bin/conductor-exec bundle exec rspec", | ||
| "lint": "bin/conductor-exec bundle exec rubocop && bin/conductor-exec yarn lint", | ||
| "archive": "" | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Shebang inconsistency: this uses
#!/usr/bin/env zshwhileconductor-setup.sh:1uses#!/bin/zsh. Should use the same approach for consistency.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI