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

Allow erb syntax on yml config file #199

Merged
merged 1 commit into from Jul 20, 2016

Conversation

Projects
None yet
2 participants
@stjhimy
Contributor

stjhimy commented Apr 28, 2016

All *.yml* config files now accepts ERB syntax. Close #116

@zzak

This comment has been minimized.

Show comment
Hide comment
@zzak

zzak May 3, 2016

Member

@stjhimy Thanks for your patch!

Doesn't this break existing applications that depend on the '.erb' suffix?

Could we support both, for now? (And add tests)

<3 <3

Member

zzak commented May 3, 2016

@stjhimy Thanks for your patch!

Doesn't this break existing applications that depend on the '.erb' suffix?

Could we support both, for now? (And add tests)

<3 <3

@stjhimy

This comment has been minimized.

Show comment
Hide comment
@stjhimy

stjhimy May 3, 2016

Contributor

Right now we support anything that matches *.yml* so it still working for both cases .yml and .yml.erb
I will add the missing spec.

Contributor

stjhimy commented May 3, 2016

Right now we support anything that matches *.yml* so it still working for both cases .yml and .yml.erb
I will add the missing spec.

@stjhimy

This comment has been minimized.

Show comment
Hide comment
@stjhimy

stjhimy May 3, 2016

Contributor

There, I kept both specs for .yml and .yml.erb files.

Contributor

stjhimy commented May 3, 2016

There, I kept both specs for .yml and .yml.erb files.

@zzak

This comment has been minimized.

Show comment
Hide comment
@zzak

zzak May 4, 2016

Member

It seems like now any file will do, that can pass the ERB and YAML parsing.

For example: config_file "app.rb" will still work, but maybe raise an exception if ERB or YAML cannot parse the document.

Member

zzak commented May 4, 2016

It seems like now any file will do, that can pass the ERB and YAML parsing.

For example: config_file "app.rb" will still work, but maybe raise an exception if ERB or YAML cannot parse the document.

@stjhimy

This comment has been minimized.

Show comment
Hide comment
@stjhimy

stjhimy May 5, 2016

Contributor

Right, should we allow only files with .yml or .yml.erb extension? Anything besides that we raise a format exception?

Contributor

stjhimy commented May 5, 2016

Right, should we allow only files with .yml or .yml.erb extension? Anything besides that we raise a format exception?

@zzak

This comment has been minimized.

Show comment
Hide comment
@zzak

zzak May 6, 2016

Member

yes, let's do that.

Member

zzak commented May 6, 2016

yes, let's do that.

@stjhimy

This comment has been minimized.

Show comment
Hide comment
@stjhimy

stjhimy May 7, 2016

Contributor

Have a look now @zzak

Thanks!

Contributor

stjhimy commented May 7, 2016

Have a look now @zzak

Thanks!

Show outdated Hide outdated lib/sinatra/config_file.rb
@@ -128,9 +128,9 @@ def config_file(*paths)
Dir.chdir(root || '.') do
paths.each do |pattern|
Dir.glob(pattern) do |file|
raise 'Invalid config file: Only .yml and .erb extensions are supported' unless ['.yml', '.erb'].include?(File.extname(file))

This comment has been minimized.

@zzak

zzak May 9, 2016

Member

I don't want to raise a plain Exception here, can we introduce a new exception class specific to this module?

For example: Sinatra::ConfigFile::UnsupportedConfigType. It should probably be nested in the module namespace since it's only going to be used in the context of ConfigFile extension.

@zzak

zzak May 9, 2016

Member

I don't want to raise a plain Exception here, can we introduce a new exception class specific to this module?

For example: Sinatra::ConfigFile::UnsupportedConfigType. It should probably be nested in the module namespace since it's only going to be used in the context of ConfigFile extension.

@zzak zzak merged commit fbfdfdc into sinatra:master Jul 20, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@stjhimy stjhimy deleted the stjhimy:config_erb branch Jul 21, 2016

zzak added a commit to zzak/sinatra-contrib that referenced this pull request Jul 22, 2016

Merge pull request #199 from stjhimy/config_erb
Allow erb syntax on yml config file
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment