The Wayback Machine - https://web.archive.org/web/20201210222013/https://github.com/rubyconfig/config/pull/142
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

Added config.prepend_sources option to Config.setup #142

Open
wants to merge 6 commits into
base: master
from

Conversation

@tjgfernandes
Copy link

@tjgfernandes tjgfernandes commented May 4, 2016

No description provided.

@pkuczynski
Copy link
Member

@pkuczynski pkuczynski commented May 12, 2016

@tjgfernandes, could you please add some documentation explaining this new feature? I honestly see no use case, so I am interested to see how you present it...

@tjgfernandes
Copy link
Author

@tjgfernandes tjgfernandes commented May 13, 2016

@pkuczynski, need to prepend custom yml setting file that depends on ENV variable to be sure that all settings dependencies are loaded based on that custom yml file. This way settings just run once, for me it's more clean/clear than using runtime approach. Also, any initializer i have can use Settings.

This way you can choose to prepend_source at config time or runtime.

@pkuczynski
Copy link
Member

@pkuczynski pkuczynski commented May 13, 2016

So the idea you have is to be able to load another settings file before loading current set of files based on the current environment? Sorry for asking, but why would you like to do that? More natural feels: load default settings and then override some of them with custom file based on ENV variable...

Could you please elaborate a little bit more on this, as I would like to better understand the reasoning before merging this into master.

@tjgfernandes
Copy link
Author

@tjgfernandes tjgfernandes commented May 13, 2016

@pkuczynski i see your point but don't see it as more natural. config.prepend_sources allows to have pre custom settings without worring to create another initializer to add_and_reload source. Configuration is done is one place and it runs once. Don't need to worry to what happens with Rails app and settings usage/dependencies in between Config initialize and source_added_and_reloaded! (even for a very short time). Settings may be updated on reload! but what happens with any config you might have passed the previous Settings value inside the Rails app?

note: The other way to do this would be to allow to config settings files at config time, but didn't have that need, settings.yml file and others are fine has they are.

@tjgfernandes
Copy link
Author

@tjgfernandes tjgfernandes commented May 13, 2016

@pkuczynski, have pull request this because i was thinking it might be a small change that could be useful. But if this isn't something you see usefull for railsconfig users, please don't merge. No problem on my side!

@pkuczynski
Copy link
Member

@pkuczynski pkuczynski commented May 13, 2016

I see your point, but I am not sure about the implementation. Let me think about it for a while and abstract it in a more general way, where you can update which files are loaded in which order in a simple and clean way. I do not like source_added_and_reloaded! myself and was planing to refactor this in the nearest future...

@pkuczynski pkuczynski force-pushed the rubyconfig:master branch from 9d92062 to 5e67861 Dec 23, 2016
@3den
Copy link

@3den 3den commented Mar 29, 2017

@pkuczynski any update on this?

This feature is very useful for my team, coz I work on apps with lots of yaml settings, that most of the time are not environment based, so we break up the settings into smaller yaml files e.g. settings/faq.yml, settings/swagger.yml, settings/data_table.yml...

Ideally we should also have an append_sources method.

@pkuczynski
Copy link
Member

@pkuczynski pkuczynski commented Mar 29, 2017

Apologies. I will have a look tomorrow.

@pkuczynski pkuczynski force-pushed the rubyconfig:master branch from eec39ab to fe60905 Feb 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.