Skip to content
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

Add basic reference information #712

Merged
merged 10 commits into from
Dec 18, 2020

Conversation

shaydewael
Copy link
Contributor

@shaydewael shaydewael commented Dec 16, 2020

Summary

This PR introduces a new reference page that includes details about listeners, initialization options, and error codes.

Live at https://shaydewael.github.io/bolt-js/reference

but you'll have to manually update the header's import of style.css you to use the shaydewael.github.io one if you want to see the styling. Or just trust me, it looks like this:
image

EDIT: Modified some styling updates re: @mwbrooks's suggestion
image

Requirements (place an x in each [ ])

@gitwave gitwave bot added the untriaged label Dec 16, 2020
@codecov
Copy link

codecov bot commented Dec 16, 2020

Codecov Report

Merging #712 (dd40a7f) into main (afe7208) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #712   +/-   ##
=======================================
  Coverage   82.32%   82.32%           
=======================================
  Files           8        8           
  Lines         758      758           
  Branches      250      250           
=======================================
  Hits          624      624           
  Misses         78       78           
  Partials       56       56           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update afe7208...dd40a7f. Read the comment docs.

@mwbrooks mwbrooks added docs M-T: Documentation work only draft A pull request that is currently in active development and not yet ready for review and removed untriaged labels Dec 16, 2020
docs/_tutorials/reference.md Show resolved Hide resolved

| Method | Description |
|--------------------------------------------------------------------|
| `app.event(eventType, fn);` | Listens for Events API events. The `eventType` is a `string` to identify a [specific event](https://api.slack.com/events) to handle (which must be subscribed to in your app's configuration). |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having a link to https://api.slack.com/events is a great idea! To avoid confusion, we may want to mention message.channels is not an event type.

docs/_tutorials/reference.md Outdated Show resolved Hide resolved
docs/_tutorials/reference.md Outdated Show resolved Hide resolved
docs/_tutorials/reference.md Outdated Show resolved Hide resolved
docs/_tutorials/reference.md Outdated Show resolved Hide resolved
docs/_tutorials/reference.md Outdated Show resolved Hide resolved
docs/_tutorials/reference.md Outdated Show resolved Hide resolved
docs/_tutorials/reference.md Outdated Show resolved Hide resolved
@shaydewael shaydewael changed the title WIP: Add basic reference information Add basic reference information Dec 17, 2020
docs/_tutorials/reference.md Show resolved Hide resolved
| :---: | :--- |
| `signingSecret` | A `string` from your app's configuration (under "Basic Information") which verifies that incoming events are coming from Slack |
| `endpoints` | A `string` or `object` that specifies the endpoint(s) that the receiver will listen for incoming requests from Slack. For `object`, the `key` is the type of events (ex: `events`), and the value is the endpoint (ex: `/myapp/events`). **By default, all events are sent to the `/slack/events` endpoint** |
| `processBeforeResponse` | `boolean` that determines whether Events API events should be automatically acknowledged. This is primarily useful when running apps on FaaS to ensure events listeners don't unexpectedly terminate. Defaults to `true`. |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd love to be able to update the Deploying to AWS Lambda - Prepare the app section to link to the processBeforeResponse documentation. 🙏🏻

### Receiver options
`ExpressReceiver` options can be passed into the app constructor, just like the Bolt app options. They'll be passed to the `ExpressReceiver` instance upon initialization.

| Option | Description |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to know which are required or optional options. I noticed that some descriptions start with "Optional,...". Would it be better to just have a column "Required"?

docs/_tutorials/reference.md Show resolved Hide resolved

> Bolt's client is an instance of `WebClient` from the [Node Slack SDK](/node-slack-sdk), so some of that documentation may be helpful as you're developing.

## Framework error types
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️ ❤️ ❤️ ❤️ ❤️ This is the kind of documentation that a developer with a production app needs and loves.

@mwbrooks mwbrooks removed the draft A pull request that is currently in active development and not yet ready for review label Dec 18, 2020
Copy link
Member

@mwbrooks mwbrooks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a first-pass, I think this is good. It'll give our community a lot of valuable content and we can continue to improve it over time.

I think we will want to tighten up the formatting style, but I don't think it should block the release.

@shaydewael shaydewael merged commit 1ddc328 into slackapi:main Dec 18, 2020
@shaydewael shaydewael deleted the add-top-level-about branch December 18, 2020 22:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs M-T: Documentation work only
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants