Skip to content

Now it's ready to be deployed to heroku#3

Merged
mvalipour merged 5 commits intomvalipour:masterfrom
Edo78:master
Jun 12, 2016
Merged

Now it's ready to be deployed to heroku#3
mvalipour merged 5 commits intomvalipour:masterfrom
Edo78:master

Conversation

@Edo78
Copy link
Copy Markdown
Contributor

@Edo78 Edo78 commented Feb 14, 2016

I've made some changes:

  • created a Procfile
  • defined a "start" script in the packages.json
  • moved the token to a env varible so the code is perfectly clean and reusable
  • used a env varible for the url used for the webhook (so the code is clean and resusable)

Let me know if something is unclear or plain wrong ;)

@volodymyrlut
Copy link
Copy Markdown

volodymyrlut commented Apr 15, 2016

@Edo78 actually we should add some kind of .enc_example file, which should not be ignored and contain example of environment configurations for those who don't know what it is.
Also, it would be great if you will notice in readme that once bot was deployed to heroku, webhook will be started and Telegram API will block pooling in development mode giving back error like
{"ok":false,"error_code":409,"description":"Error: Conflict: another webhook is active"}
This is a lil bit confusing for the beginners, because we use pooling in the dev environment and webhooks in the production and Telegram is not starting the polling process once existing webhook is running.
So we should tell user to remove active webhook, using the URL https://api.telegram.org/bot(bot token here)>/setWebhook?url=

@Edo78
Copy link
Copy Markdown
Contributor Author

Edo78 commented Apr 15, 2016

Actually since I made the PR I stop even using the bot (it should still be on heroku) and I barely remember anything. I have to look at the code and at the telegram bot documentation.

@mvalipour
Copy link
Copy Markdown
Owner

Hi @Edo78

Thanks for the great work. What you've done is the right thing to do indeed! -- the code was originally done as a very minimalistic example.

I'd like to see best practices followed and visible even in the most hacky codes of us, so thumbs up!

@mvalipour mvalipour self-assigned this Jun 12, 2016
@mvalipour mvalipour merged commit 3f9d540 into mvalipour:master Jun 12, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants