search

codeforamerica / ohana-api

The open source API directory of community social services.
http://ohana-api-demo.herokuapp.com/api
BSD 3-Clause "New" or "Revised" License
185 stars 344 forks source link

Added platform configuration helper #420

Closed CristianCurteanu closed 6 years ago

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.005%) to 99.373% when pulling 760578a72d8fdd40af492cc0b929800d6e0e569c on CristianCurteanu:small-fixes into e337b7e84e16dde5e836b58de8be3cb1cba460a1 on codeforamerica:master.

monfresh commented 7 years ago

Thanks for your contribution. Could you please explain what problem this PR is solving?

CristianCurteanu commented 7 years ago

I decided it could be handy to configure some parts of app from YAML files, as in this case to extract hardcoded URLs into a config file. But it can be useful for other things (ex. flags)

monfresh commented 7 years ago

I agree that it makes sense to allow some parts of the app to be configured via YAML, such as application.yml, settings.yml, and i18n. However, I don't think the JSON for the root endpoint is worth making customizable in this way.

I find this code is a lot harder to read. I don't think it's worth adding 30 lines of code with very little difference in customizability. Is the difference between editing the root_controller.rb file versus the YAML file big enough to justify the additional complexity? I would say no, but I'm open to hearing convincing arguments if you have any.

I also don't expect most people will want to customize this. If they do, they will also need to update the routes.rb file, which means they are technically savvy, and if they are, they should be able to edit the root_controller.rb file directly.

I'm curious to know whether you had a need to customize the endpoints, and if so, in what way?

monfresh commented 6 years ago

Closing this due to inactivity.