make all nconf options can be overwrite by environment variables.

[hychen]

the idea is generating default values of nconf by using makeDefaultConfig function, and makeDefaultConfig sets proper values if environment is present or not.

makeDefaultConfig receives an object includes pairs of environment variables and default values.

Impact

rename DB_PATH to DATABASE_PATH , so user can user DATABASE_PATH to overwrite {'database': {'path':''}} rename twitter_url to twitter_callback in nconf, so user can overwrite it by setting TWITTER_CALLBACK environment variable.

[rufuspollock]

Looks good but not quite sure of purpose. How does this improve on the stuff in lib/config.js where we do e.g.

"backend": process.env.BACKEND || "s3"

(i.e. use s3 as default and allow override by environment variable)

[hychen]

In master branch, not all nconf options can be override by environment variable, but it said "Set specific environment variables. .... The ones you can set are those used in nconf.defaults."

I start to made this patch when I found db.backend can not be override by environment variable, so I sent a pull request (#142)

but developers may not remember to add the statement of "process.env.$VAR || default value" one by one when he adds a new options.

this is the major purpose of this patch - to make sure all options of nconf can be override by environment variable.

besides, with this modification, there is a corresponding method between the environment name and the name of options in settings.josn.

so, we can simply introduce available environment names that users can use and the transform way if he prefer to write down the config in settings.json.

my 2 cents :)

Best regards, Hsin Yi Chen

More about me: http://about.me/hychen

2014-06-12 16:32 GMT+08:00 Rufus Pollock notifications@github.com:

Looks good but not quite sure of purpose. How does this improve on the stuff in lib/config.js where we do e.g.

"backend": process.env.BACKEND || "s3"

(i.e. use s3 as default and allow override by environment variable)

— Reply to this email directly or view it on GitHub https://github.com/okfn/timemapper/pull/149#issuecomment-45842815.

[rufuspollock]

Makes load of sense - thanks for clarifying. I'm going to merge - may be worth putting a short comment on the main method you have to make this clear - it was a little bit cryptic (that's why i asked)

[rufuspollock]

And please keep these excellent patches coming :-)