Back to all reviewers

Structured environment configuration

Homebrew/brew
Based on 4 comments
Ruby

Use structured environment variable handling with explicit validation instead of directly accessing ENV hash. Implement proper parsing for boolean values, default values, and type conversion. Consider making paths user-specific when creating temporary configurations.

Configurations Ruby

Reviewer Prompt

Use structured environment variable handling with explicit validation instead of directly accessing ENV hash. Implement proper parsing for boolean values, default values, and type conversion. Consider making paths user-specific when creating temporary configurations.

For boolean environment variables, handle falsy values properly:

# Instead of this:
if ENV["HOMEBREW_SOME_FLAG"].present?
  # Do something
end

# Do this:
if Homebrew::EnvConfig.some_flag?
  # Do something
end

# When implementing environment variable handlers:
def env_method_name(env, hash)
  if hash[:boolean]
    define_method(method_name) do
      env_value = ENV.fetch(env, nil)
      
      falsy_values = %w[false no off nil 0]
      if falsy_values.include?(env_value&.downcase)
        false
      else
        ENV[env].present?
      end
    end
  end
end

For configuration paths that may be shared across users, add user-specific identifiers:

# Instead of this:
zdotdir = Pathname.new(HOMEBREW_TEMP/"brew-zsh-prompt")

# Do this:
zdotdir = Pathname.new(HOMEBREW_TEMP/"brew-zsh-prompt-#{Process.euid}")

This approach provides consistent interpretation of configuration values, reduces bugs from unexpected environment variable content, and prevents permission issues with shared configuration paths.

4
Comments Analyzed
Ruby
Primary Language
Configurations
Category

Source Discussions