store the actual feature configuration in a variable instead of in real constants under the Feature module#4
Conversation
…al constants under the Feature module
att14
left a comment
att14
left a comment
There was a problem hiding this comment.
Looks good. Nothing major jumps out.
| @@ -1,3 +1,5 @@ | |||
| require 'spec_helper' | |||
There was a problem hiding this comment.
This should happen automatically from the .rspec file.
There was a problem hiding this comment.
Yeah, I added them when I was trying to fix some test failures (which actually turned out to be because the describe Feature::Foo declaration was trying to load the constant Feature::Foo before the before(:each) block ran, which I fixed by changing all of the describes from constants to strings. I'll remove them.
| base = path.chomp(File.extname(path)).split("/") | ||
| if base.size > 1 | ||
| directories = base[0...-1] | ||
| filename = base[-1] |
There was a problem hiding this comment.
you know me. explicit > implicit.
| def reinit_if_changed | ||
| # Reload the configuration if the top-level directory has changed. | ||
| # Does not detect changes to files inside that directory unless your | ||
| # |
There was a problem hiding this comment.
This comment isn't complete, but I think it is in the README.
|
|
||
| @@tree = Module.new | ||
|
|
||
| def Feature.set_tree(tree) |
There was a problem hiding this comment.
Can you just use self here instead of Feature?
- Re-running `Toggles.init` will now correctly re-load all features, including removing features that have been removed from the underlying store - New `Toggles.reinit_if_changed` method will cache the inode/mtime of the root directory that feature config lives in and run `init` if and only if that directory itself has changed - Various test improvements Merge remote-tracking branch 'origin/store_in_variable' Closes #4
2 participants
Right now, if you call
Toggles.initafter toggles has already been initialized, it will leave the old config in place (it has no way of removing previous definitions).This instead builds the entire class hierarchy in an anonymous module and attaches it as a variable to the
Featuresmodule, meaning that it can be atomically swapped into and out of place.I still think that using the module system to store dynamic data is very poor taste and if I felt like refactoring all of our internal consumers, I would probably rewrite this to explicitly use nested hashes.