-
-
Notifications
You must be signed in to change notification settings - Fork 234
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
Kernel.const_set, seriously? #122
Comments
How would you suggest to implement it differently? |
Anyone who needs this to be loaded into the global namespace by another name should just do it themselves. It's a trivial assignment. require "config"
MySettings = Config I suggest removing this feature. |
Through definition a custom class or module: class Settings < Config::Base
end or class Settings
include Config
end |
You are totally right that Kernel.const_set is not the best way to expose settings. This was in the code, before I joined the project and I never really thought about it before... There are couple of problems with the current setup actually. First of all Config is a module which holds the logic of loading settings, while what you referring to is a static OpenStruct object called Settings, holding all the values for settings - so they two different things and neither of the solutions you described above would work. I actually never liked this split and always thought it should be a single object. Additionally its confusing with config as a gem name and object used to wire things up, while actually values are accessed via Settings instance. Config also conflicts with some old object name and raise a warning... I do not have anything fixed in mind how to solve this, as I assume original gem author wanted to keep Settings slim... I will have to think about it... Any suggestions welcome... |
Regarding the problem of exposing
|
Fixed |
The Kernel module is included by class Object, so its methods are available in every Ruby object.
Result:
The text was updated successfully, but these errors were encountered: