Open danalexilewis opened 4 years ago
I'm on a call - will review later today (straight after call if it doesnt run into my next meeting).
So @mitra42 @danaronson I'd like to expand on the readme and then merge this code back into master, so we don't have config floating around outside of our core trunk. Is that agreeable?
absolutely
Sorry - I crossed in the post - with @danaronson. I agree that check for a env variable ahead of a config variable makes sense to me.
@agentlewis where were you setting ON_HEROKU ? Or is that an environmental variable set globally by Heroku (I've never used Heroku)
can you do the PR to develop instead of master?
@danaronson I presume your last message was to @agentlewis ? Note that I merged it into Develop a while back, so I think this PR could be deleted.
But - you made a suggestion for a code change above :
port = int(os.environ.get('PORT', config.get('port', 8080))
which isn't incorporated.
The readme can be tidied up and the commits squashed (they are messy as I needed to commit to test the github->heroku integration)
But these are the changes I made to get it to work.
Key noticing is that once hosted on heroku we don't need to specify the port in the url - I assume the heroku router takes care of that.
So @mitra42 @danaronson I'd like to expand on the readme and then merge this code back into master, so we don't have config floating around outside of our core trunk. Is that agreeable?