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 ability for app to run without application.yml file #907

Merged
merged 3 commits into from
Dec 29, 2016

Conversation

patcon
Copy link
Contributor

@patcon patcon commented Dec 28, 2016

Currently, app can't run on Heroku unless application.yml is committed,
whereas the preferred method of working is by setting envvars in .env.

@patcon
Copy link
Contributor Author

patcon commented Dec 28, 2016

Related to #903

Currently, app can't run on Heroku unless application.yml is committed,
whereas the preferred method of working is by setting envvars in `.env`.
@monfresh
Copy link
Contributor

Good point. We upload application.yml to our servers via Chef, but we shouldn't assume that everyone will do this. We should come up with a more generic solution that uses the ENV keys. This should work:

class FigaroYamlValidator
  ENV_KEYS = ENV.keys.delete_if { |key| key.include?('_FIGARO') }

  def validate(keys = ENV_KEYS)
    bad_keys = []
    check_for_bad_values(keys, bad_keys)
    return unless bad_keys.any?
    raise warning(bad_keys).tr("\\\n", ' ')
  end

  private

  def check_for_bad_values(keys, bad_keys)
    keys.each { |key| bad_keys << key if %w(yes no).include?(ENV[key]) }
  end

  def warning(bad_keys)
    <<~HEREDOC
      You have invalid values for #{bad_keys.uniq.to_sentence} in
      config/application.yml. Please change them to 'true' or 'false'
    HEREDOC
  end
end

We should also probably rename the class to something like ConfigValidator.

@monfresh
Copy link
Contributor

@patcon What do you think about my solution? If you like it, wanna update your PR to incorporate it?

@patcon
Copy link
Contributor Author

patcon commented Dec 29, 2016

Slightly different so that spec works cleanly. Lemme know if you'd like a different approach!

patcon added a commit to patcon/id.c4nada.ca that referenced this pull request Dec 29, 2016
@monfresh
Copy link
Contributor

LGTM. Could you please address the Code Climate issue? Not sure if you can see what it is. It's complaining about the comma at the end of the hash on line 15 of the spec.

@monfresh
Copy link
Contributor

Looks great, thanks!

Copy link
Contributor

@monfresh monfresh left a comment

Choose a reason for hiding this comment

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

👍

@monfresh monfresh merged commit f9c4318 into 18F:master Dec 29, 2016
amoose pushed a commit that referenced this pull request Feb 24, 2017
* Allow app to run without application.yml file

**Why**: Currently, app can't run on Heroku unless application.yml is 
committed, whereas the preferred method of working is by setting env 
vars.
amoose pushed a commit that referenced this pull request Feb 28, 2017
* Allow app to run without application.yml file

**Why**: Currently, app can't run on Heroku unless application.yml is 
committed, whereas the preferred method of working is by setting env 
vars.
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

2 participants