-
Notifications
You must be signed in to change notification settings - Fork 399
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[#220] Add deployment guide for Heroku #651
[#220] Add deployment guide for Heroku #651
Conversation
Codecov Report
@@ Coverage Diff @@
## main #651 +/- ##
=======================================
Coverage ? 84.41%
=======================================
Files ? 8
Lines ? 693
Branches ? 206
=======================================
Hits ? 585
Misses ? 71
Partials ? 37 Continue to review full report at Codecov.
|
docs/_deploying/heroku.md
Outdated
First, select **Interativity & Shortcuts** from the side and update the **Request URL**: | ||
|
||
![Interactivity & Shortcuts page](../assets/interactivity-and-shortcuts-page.png "Interactivity & Shortcuts page") | ||
|
||
Second, select **Event Subscriptions** from the side and update the **Request URL**: | ||
|
||
![Event Subscriptions page](../assets/event-subscriptions-page.png "Event Subscriptions page") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shaydewael I added two images to help explain where to update the Request URL in the app management page. I know you're thinking about improving the images in the Getting Started guide. I don't want these images to step on the toes of your work, so let me know if there's a better approach that you have in mind.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is already a great guide 👍
docs/_includes/sidebar.html
Outdated
@@ -63,6 +60,18 @@ | |||
</ul> | |||
<!--End (temporary) beta sections--> | |||
|
|||
<ul class="sidebar-section"> | |||
<a href="{{ site.url | append: site.baseurl | append: localized_base_url }}/deploying"> | |||
<li class="title">{{ site.t[page.lang].deploying }}</li> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should make this section header a conditional until there's a translated version
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added commit cef89ab to only display the Deployments section when there is content (including hiding the section header). Currently, this will hide the Deployments section for the Japanese translate.
@shaydewael my Jekyll/Liquid skills 🔪 are a little rusty. Would you mind looking at my implementation? If you have a more elegant way to do it, I'd love to see it!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great and so important. It's going to help provide such a seamless experience for people that want to use Heroku to host. 🎉
… in lang This will hide the section for Japanese documentation until there is a translation available. - See slackapi#651 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regarding the place to have the example app, as I mentioned in a comment, my suggestion is to have it under almost unused examples
dir in this repo. But other options are also great. Apart from that, all look great to me.
- See slackapi#651 (comment) - See slackapi#651 (comment) - See slackapi#651 (comment) - See slackapi#651 (comment) - See slackapi#651 (comment) - See slackapi#651 (comment) - See slackapi#651 (comment) - See slackapi#651 (comment) - See slackapi#651 (comment) - See slackapi#651 (comment) - See slackapi#651 (comment)
- See slackapi#651 (comment) - See slackapi#651 (comment) - See slackapi#651 (comment) - See slackapi#651 (comment) - See slackapi#651 (comment) - See slackapi#651 (comment) - See slackapi#651 (comment) - See slackapi#651 (comment) - See slackapi#651 (comment)
… in lang This will hide the section for Japanese documentation until there is a translation available. - See slackapi#651 (comment)
- See slackapi#651 (comment) - See slackapi#651 (comment) - See slackapi#651 (comment) - See slackapi#651 (comment) - See slackapi#651 (comment) - See slackapi#651 (comment) - See slackapi#651 (comment) - See slackapi#651 (comment) - See slackapi#651 (comment) - See slackapi#651 (comment) - See slackapi#651 (comment)
- See slackapi#651 (comment) - See slackapi#651 (comment) - See slackapi#651 (comment) - See slackapi#651 (comment) - See slackapi#651 (comment) - See slackapi#651 (comment) - See slackapi#651 (comment) - See slackapi#651 (comment) - See slackapi#651 (comment)
f650ef1
to
bb88411
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
approved again - great work!
Summary
This pull request is part of the on-going work related to #220. After reviewing, we will want to squash this merge.
This pull request adds the following:
bolt-js/examples/
Remaining tasks before merging:
After merging, we will want to:
Preview
→
https://mwbrooks.github.io/bolt-js/deployments/herokuRequirements (place an
x
in each[ ]
)