search

LARailsLearners / box-office

2 stars 1 forks source link

gets info from route #2

Open kangkyu opened 7 years ago

kangkyu commented 7 years ago

open localhost:4567/tt0078346 then the page says "Superman"

thomasjinlo commented 7 years ago

Does anyone need the API key for TMDb?

kangkyu commented 7 years ago

don't we? it's in your code, as ENV and you need yours if run it locally for development

thomasjinlo commented 7 years ago

I meant I can provide my API key if people don't want to sign up for a new one

kangkyu commented 7 years ago

But still, it's public repo so we better not include your own API key to readme or in the code. Still we need some instruction how to develop and have value on ENV['TMDB_API_KEY'] I think...

server-monitor commented 7 years ago

API keys are usually stored on some env manager. Each developer usually has his own set of keys then the keys are loaded using env manager. I usually use Figaro in Rails, then my keys and other data I don't want committed to the repo are stored in config/application.yml

On Sun, Aug 14, 2016 at 10:38 PM, Kang-Kyu Lee notifications@github.com wrote:

But still, it's public repo so we better not include your own API key to readme or in the code. Still we need some instruction how to develop and have value on ENV['TMDB_API_KEY'] I think...

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/LARailsLearners/box-office/issues/2#issuecomment-239732728, or mute the thread https://github.com/notifications/unsubscribe-auth/ARG9m6DfQv5_nIKeizp9Xv9cND2d2jzvks5qf_tXgaJpZM4Jj0xV .

kangkyu commented 7 years ago

yes that's better, then you can do that in your commit of this PR #14 ✨

server-monitor commented 7 years ago

I'm going to do some research first on how to integrate Figaro into Sinatra. It shouldn't be too bad.

kangkyu commented 7 years ago

Did you close this issue because we're not implementing this any more @btmash ? Come to our slack chat https://laruby.slack.com/messages/eastside_study_group/

kangkyu commented 7 years ago

@thomasjinlo @server-monitor now I see there's open issue https://github.com/LARailsLearners/box-office/issues/11 and we better add dotenv (or figaro) as a separate PR. Anyways we can finish this issue first without it

server-monitor commented 7 years ago

I did a little research last night and it seems dotenv is easier to integrate with Sinatra than Figaro so we'll go with dotenv if there are no objectsions