Skip to content
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

add a way to prefer environment over settings #16

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

colstrom
Copy link

Describe the change

Adds an (optional) way to prioritize environment over settings.

This is done with the following changes:

  • Adds a @preferred instance variable, getter, and a validating setter. It takes one of two values:
    • :settings (default) - this is how it currently works without this change: check settings first, falling back to environment.
    • :environment - reverses the standard lookup order: checking environment first, then settings as a fallback.
    • Adds prefer(source) as an alias for preferred=(source), because it reads nicely.
  • Adds a TTY::Config::UnsupportedSource exception for when told to prefer an unrecognized source.
  • Adds a TTY::Config.normalize_preferred(source) method that allows several intuitive source names to be normalized to one of the correct values:
    • settings, configuration, config, file, and files all become settings, because that seems to be what TTY::Config refers to the config file values as.
    • environment, env, and ENV all become environment.
  • Adds an (optional) keyword argument to TTY::Config#fetch: prefer, allowing this to be overridden on a per-fetch basis if desired.

The naming of these things can of course be adjusted if they're not internally consistent with TTY's naming style.

When selecting the aliases above, consideration was given to how the code would read in both contexts:

config = TTY::Config.new do |config|
  config.preferred = :configuration
  config.prefer :files # IMO, this reads better, but is equivalent to the above, since conventional setter names are somewhat expected.
  config.prefer :environment
end

config.fetch("foo.bar", prefer: :settings)

Since the : : bit is a bit ugly (IMO), the option can be passed as a String (technically anything that responds to to_sym, but I'm really not sure what else one would WANT to pass in... and either way the validation will catch it).

Why are we doing this?

From my perspective, I usually want environment to take priority over a config file, if a value is defined in both. Kind of like how I want options given on the command line to take priority over either, y'know? In my head, it's something like this:

  • Config files can be shared by multiple contexts (such as shell sessions) on the same system.
  • Environment can be shared by multiple instances of a program, and are thus (generally) more context-specific than config files.
  • Options are specific to a single instance (granted, options are not covered by this gem, that's a tty-option thing).

I recognize that what's intuitive to me, may not align with others, so this change leaves the choice up to the developer.

Benefits

Allows developers using tty-config to choose whichever order suits their needs.

Drawbacks

One more option to configure, I suppose. But it's optional, and it defaults to working the way it does already, so I don't see any obvious drawbacks.

Requirements

  • Tests written & passing locally?
  • Code style checked?
  • Rebased with master branch?
  • Documentation updated?
  • Changelog updated?

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.

None yet

1 participant