Leaderboard Deploy agents
Back to all reviewers

avoid environment variable proliferation

block/goose
Based on 6 comments
Rust

Prefer the consolidated config system over adding new environment variables. Environment variables should only be used for deployment-specific overrides, not as the primary configuration mechanism.

Configurations Rust

Reviewer Prompt

Prefer the consolidated config system over adding new environment variables. Environment variables should only be used for deployment-specific overrides, not as the primary configuration mechanism.

Guidelines:

  • Use the existing Config::global().get_param() system for new configuration options
  • Pass configuration values as explicit parameters to functions rather than accessing global config deep in the call stack
  • Environment variables should override config file values, maintaining consistent precedence
  • Avoid adding new GOOSE_* environment variables unless absolutely necessary for deployment scenarios

Example:

// ❌ Avoid: Adding new environment variables
const GOOSE_RECIPE_RETRY_TIMEOUT_SECONDS: &str = "GOOSE_RECIPE_RETRY_TIMEOUT_SECONDS";

fn get_retry_timeout(retry_config: &RetryConfig) -> Duration {
    let timeout_seconds = env::var(GOOSE_RECIPE_RETRY_TIMEOUT_SECONDS)
        .ok()
        .and_then(|s| s.parse().ok())
        .unwrap_or(DEFAULT_RETRY_TIMEOUT_SECONDS);
    Duration::from_secs(timeout_seconds)
}

// ✅ Prefer: Use config system with parameter passing
fn get_retry_timeout(retry_config: &RetryConfig, config_timeout: Option<u64>) -> Duration {
    let timeout_seconds = retry_config.timeout_seconds
        .or(config_timeout)
        .unwrap_or(DEFAULT_RETRY_TIMEOUT_SECONDS);
    Duration::from_secs(timeout_seconds)
}

// At call site:
let config = Config::global();
let timeout = config.get_param("RETRY_TIMEOUT_SECONDS").unwrap_or(DEFAULT_RETRY_TIMEOUT_SECONDS);
get_retry_timeout(&retry_config, Some(timeout))

This approach reduces environment variable sprawl while maintaining flexibility for deployment-specific configuration through the existing config system.

6
Comments Analyzed
Rust
Primary Language
Configurations
Category

Source Discussions