custom autologin cookie name not read (Bug #273)

If I didn't know better I'd say Redmine::Configuration is a bad joke -_-.

Anyway, I've updated your patch and made a pull request with it at https://github.com/chiliproject/chiliproject/pull/30.

Now, this had me working with Redmine::Configuration, which strikes me as half-assed at best. While having a central configuration always reminds me of the often-loathed windows registry, I can also see that it has advantages.

What's happening with Redmine::Configuration though apparently is that every place that reads a value from there also has to define a default in-place (cookie_name = Redmine::Configuration['autologin_cookie_name'] || 'autologin'), meaning if you every want to change that default, you'd have to go through everywhere this configuration piece is used and change it. Not nice.

You'd think there's a place to put defaults for those, you open the corresponding file and oh joy, you stumble over a hash named @defaults (source:/lib/redmine/configuration.rb#L21) with the comment "Configuration default values". Looks exactly like what you'd need, right? Well, nearly, the problem is that every key in source:/config/configuration.yml.example is in there uncommented, meaning it gets read with a value if nil, which in turn overwrites the so-called "defaults" from @defaults. To make matters worse, the "base" or "common" section in the source:/config/configuration.yml.example that gets loaded before any environment-specific configuration is called default too, and that gets patched together programmatically in Redmine::Configuration, instead of leveraging similar functionality provided by YAML directly (though I don't know whether the YAML-one does a flat or a deep merge…).

Anyway, all this ranting for this:

• Should we consider key with a nil value in the configuration file as unset?
• Should we keep it as-is and uncomment the keys in source:/config/configuration.yml.example so that the @defaults in source:/lib/redmine/configuration.rb#L21 really is a default config?
• Should we keep the defaults at the place where the variable gets used?

And on a more philosophical/semantic matter:

• Can we rename the "default" section in the config file to "common"?
• Should we switch the config file to using yaml references instead of merging it in in source:/lib/redmine/configuration.rb?

Felix Schäfer wrote:

If I didn't know better I'd say Redmine::Configuration is a bad joke -_-.

Anyway, I've updated your patch and made a pull request with it at https://github.com/chiliproject/chiliproject/pull/30.

Pull request looks good to me as long as the tests are passing

Now, this had me working with Redmine::Configuration, which strikes me as half-assed at best.

We should discuss this in the forums. From what I saw Configuration included some of Setting which has it's own set of edge cases. I'm in favor of refactoring it if it improves the internal design.