Yash: ability to load config files through the magic of hashie :)

[gregory]

Still need to update the changelog and readme... and squash :)

how you guys like it? cc @dblock

[dblock]

I love it.

I have some reservations about calling it "Fish", since that would be the first class that doesn't end with "ash". It could be called "Flash", especially that flash is a kind of storage. Another, possibly better name could be "Cash", for configuration. What do you think?

[gregory]

@dblock go for cash (config-hash) since it's more about config files than a storage system ... and we could keep flash for a futur usage.

what do i think? i love it of course :D i've update the changelog & readme.

[bartoszkopinski]

I'd be confused by Cash, that's a good name for a something related to money.

For a Yaml handling class I'd go for something like Yash maybe? A *.yml file doesn't have to be a config.

[gregory]

I don't mind the name. Yash sounds good to me too... @dblock or anyone reading this, should we go with Yash?

[bartoszkopinski]

As a side note, I don't think I get the use of ['setting'] (should be "settings" I guess) in this example:

  #   if you have a config file like the following:
  #   #/etc/config/setting.yml
  #   development:
  #     foo: 'bar'
  #   production:
  #     foo: <%= ENV['FOO'] %> #let's say that ENV['FOO'] is set to 'BAR'
  #
  #   You can start cashing like this:
  #   config = Cash.new(folder_path: '/absolute_config_path')
  #   config['setting'].development # =>{ foo: 'bar' }

• Mash is all about avoiding those brackets so I'd prefer config.settings.development instead, even in examples.
• The param is called cash_alias_method, but that's not really an "alias method", is it? More like an additional namespace or reader accessor.
• Could this be optional in a way that allows you to set no "alias" and use config.development.foo.bar directly? See Konf gem.

Just my thoughts, but anyways, very nice job!

B.

[gregory]

could be confusing, but the config['setting'](which should be settings:) is to access the setting.yml config file.
You feel like config.setting might be less confusing about what is happening?

cash_alias_method is indeed not an alias method, but the method you'll call to access your settings (by default: settings). any suggestion to rename that field?

The diff with the Konf gem, is that with one single module you can access any config file...

config = Cash.new(folder_path: '/absolute_config_path', namespace: 'production')
config['twitter'].api_key # => "api key for prod"
config['facebook'].client_id # => "facebook api key for prod"

Though i could do something like this:

twitter_config = Cash.new(folder_path: '/absolute_config_path', filename: 'twitter.yml', namespace: 'production')
twitter_config.api_key # => "Api key for prod"

[bartoszkopinski]

Thanks for clearing that up. I get the difference now.
Well it's fine with me, just maybe add it to the Readme so it's clear that "setting" is a filename and "settings" is a default alias method for extending.

As for proposed future changes:

• I'd stick with methods instead of brackets for consistency with Mash.
• For a single file you can always detect it and handle both cases

Yash.new('/path', :production)
Yash.new('/path/config.yml', :production)

[dblock]

Ok, I think @bartoszkopinski is right regarding the naming, I'm down with Yash. I also do think the syntax of new above follows the rule of least surprise. It also makes Yash's default behavior very specific for YAML configuration files, which I think is good. I'll review the rest inline.

[dblock]

I think I would rename this to settings as suggested. Or maybe give it a completely different name, like database, just for illustration purposes and borrow variable names from a typical database configuration, like say host and port instead of foo.

[dblock]

Thanks for being patient on this @gregory. I think we have an opportunity to build the best YAML settings reader here, so lets try to get this right :)

[gregory]

doing a little refacto, will push when i'm done

[gregory]

@dblock & @bartoszkopinski i made some big changes on my vision of this. check the readme and let me know what you guys think about it! (personally , i love this new approach!)

[dblock]

Should say Yash.

[dblock]

I love how this implementation has evolved, see my comments above.

[gregory]

@dblock what do you think about this?

[dblock]

I think I like.

[gregory]

ready for review and merge @dblock ;)

[dblock]

I think this should be a hash, parser: MyCustomCsvParser to enable future options or passing random options to passers.

[dblock]

Oh I think that's what's implemented, just a doc bug.

[gregory]

@dblock updated ;)

[dblock]

Rebased and merged via 59069f1.