johnkferguson / octomaps

Octomaps displays all contributors to an open-source project on a map based upon the contributor's location.
http://octomaps.com/
MIT License
15 stars 3 forks source link

Externalize database configuration and add Travis support. #28

Closed dblock closed 10 years ago

dblock commented 10 years ago

This makes the database configurable via config/database.yml, which is simpler to manage and easier on the eyes. The .yml should never need to be modified, really, and if needed it's read with an ERB template, so you can write ENV values in it.

Added Travis CI to run tests to make sure that works correctly. Before merging, enable travis on travis-ci.org for this repo. There's a successful build in https://travis-ci.org/dblock/octomaps/builds/18555022, the badge will look like this: Build Status

It looks like the production deployment has a shared database.rb. That will need to change to a shared database.yml. I couldn't test it, but it looks like it will work.

johnkferguson commented 10 years ago

Thanks @dblock. I'm gonna see if I can get dotenv working before I merge this, that way I'll be able to keep all my database credentials intact.

Previously, I tried getting Octomaps setup with dotenv but it would only work in development. In production, I would encounter an error when requiring dotenv where it would say it could not load the file.

dblock commented 10 years ago

If you have DATABASE_URL in your .env, this just works, no? Or the database name is different and you need to default it? I guess my question is: what keys do you have/want in your .env?

johnkferguson commented 10 years ago

My initial concern was that I would lose access to my database username and password, since I had been previously defining it in my database.rb file and then having it symlinked over on deploy. However, I don't think that's actually an issue since you updated the deploy task to symlink database.yml. What that means is I can just create a database.yml file with all the same info on the server, and it will be symlinked over as well.

So this should be all good to merge. Thanks! :smiley: