Closed Liz4v closed 7 years ago
Ooooh. This is awesome. Thank you so much for contributing this. I'll review and test and merge this soonish. Thanks again!
Do you have any questions for clarification?
Nope. No questions. It looks super straight forward. I just haven't had a chance to run it for myself to test. Will do so pronto. Thank you so much for contributing and for the reminder. On Tue, Sep 12, 2017 at 5:12 PM Ekevoo notifications@github.com wrote:
Do you have any questions for clarification?
— You are receiving this because you commented.
Reply to this email directly, view it on GitHub https://github.com/tommeagher/heroku_ebooks/pull/28#issuecomment-328985775, or mute the thread https://github.com/notifications/unsubscribe-auth/AArkMmvrsBXxOa89ByZ1P7JyFMQs1c4uks5shvOhgaJpZM4OgC_R .
This looks great and seems to work fine. @ekevoo, should we update the runtime.txt or remove it? Is it also worth mentioning in the README that it now supports Python3, too?
I agree with updating the readme-- that didn't occur to me at all.
I would've coded a bit differently if the plan was upgrading the runtime to Python 3. I did it pretty py-2 centric. But that's just semantics, it works nonetheless, so it's a fine idea, too.
On Sep 26, 2017 13:23, "Tom Meagher" notifications@github.com wrote:
This looks great and seems to work fine. @ekevoo https://github.com/ekevoo, should we update the runtime.txt or remove it? Is it also worth mentioning in the README that it now supports Python3, too?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/tommeagher/heroku_ebooks/pull/28#issuecomment-332272554, or mute the thread https://github.com/notifications/unsubscribe-auth/AA1NItPDB6VoAzxHS6USYT_yBpZisAIUks5smTMjgaJpZM4OgC_R .
@ekevoo do you want to take a crack at making those tweaks?
Yeah sure! I'll do that in the weekend :)
On Sep 27, 2017 10:43 AM, "Tom Meagher" notifications@github.com wrote:
@ekevoo https://github.com/ekevoo do you want to take a crack at making those tweaks?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/tommeagher/heroku_ebooks/pull/28#issuecomment-332544658, or mute the thread https://github.com/notifications/unsubscribe-auth/AA1NImLXlEPtWTNotbP-VGlhepDsivb_ks5sml8YgaJpZM4OgC_R .
I couldn't find anything in the README on which version of Python it uses, so I didn't touch that.
Merged this. Thank you, @ekevoo!
I know that
runtime.txt
explicitly asks for 2.7.13, but I happen to not have 2.7 locally and adding Python 3 support was easy and unobtrusive enough. Well, at least I thought it was. ;)