-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
chore(slack): Add slack manifest for easy slack app creation #15088
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
base: master
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
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.
good call! Thank you for doing this, just some minor comments about defaulting to these docs as being for self-hosted, with notes for what employees should do differently.
"slash_commands": [ | ||
{ | ||
"command": "/{YOUR_NAME}-sentry", | ||
"url": "https://{YOUR_DOMAIN}/extensions/slack/commands/", |
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.
The usage of YOUR_DOMAIN
later on in the page don't include the protocol https://
at the start, so I'd either remove them here, or adjust them all over -- though I think removing them is consistent with other integrations.
**NOTE**: If you choose to use the manifest provided, you must still configure your sentry environment with the slack app's Client Secret, Client ID and Signing Secret as directed in [From Scratch](#from-scratch) | ||
|
||
#### From Manifest | ||
Make sure to replace all instances of `{YOUR_NAME}` and `{YOUR_DOMAIN}`! |
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.
All of the YOUR_NAME
stuff is meant for internal sentry devs only, but these docs are also the same ones that Self-Hosted customers will use. I think it's worth having the regular, production names/commands (e.g. Sentry
, Sentry
, /sentry
) in the JSON blob, but adding a note for employees.
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.
Actually, looks like we do this on Line 200, maybe copy that over here (but also 🥔 if you adjust the table down there to also default to the production command)
- `/{yourname}-sentry`
+ `/sentry`
 | ||
|
||
### Ways to Create a Slack App | ||
There will be two options, `From a manifest` and `From scratch`, we have provided a manifest template below for convenience. |
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.
There will be two options, `From a manifest` and `From scratch`, we have provided a manifest template below for convenience. | |
There will be two options: `From a manifest` and `From scratch`, we have provided a manifest template below for your convenience. |
|
||
### Ways to Create a Slack App | ||
There will be two options, `From a manifest` and `From scratch`, we have provided a manifest template below for convenience. | ||
**NOTE**: If you choose to use the manifest provided, you must still configure your sentry environment with the slack app's Client Secret, Client ID and Signing Secret as directed in [From Scratch](#from-scratch) |
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.
**NOTE**: If you choose to use the manifest provided, you must still configure your sentry environment with the slack app's Client Secret, Client ID and Signing Secret as directed in [From Scratch](#from-scratch) | |
**NOTE**: If you choose to use the manifest provided, you must still configure your sentry environment with the slack app's Client Secret, Client ID, and Signing Secret as directed in [From Scratch](#from-scratch) |
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.
LGTM!
DESCRIBE YOUR PR
For notification platform cli tool, unfortunately users currently need to create a slack app to test their notifications via cli, so this makes a little easier with a pre-built manifest to speed up app creation. Adds a toggle :0) and a bit more info on ways to create a slack app.
Toggle & Images

IS YOUR CHANGE URGENT?
Help us prioritize incoming PRs by letting us know when the change needs to go live.
SLA
Thanks in advance for your help!
PRE-MERGE CHECKLIST
Make sure you've checked the following before merging your changes: