-
Notifications
You must be signed in to change notification settings - Fork 657
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 Support for App Manifest Endpoints #1355
Add Support for App Manifest Endpoints #1355
Conversation
The changes look good to me. If you have more time, can you add a test script under const { WebClient } = require('@slack/web-api');
const options = {};
if (process.env.SLACK_SDK_TEST_DEV_API_URL !== undefined) {
options.slackApiUrl = process.env.SLACK_SDK_TEST_DEV_API_URL;
}
const client = new WebClient(process.env.SLACK_SDK_TEST_TOOLING_TOKEN, options);
// perform API calls below ... |
options.slackApiUrl = process.env.SLACK_SDK_TEST_DEV_API_URL; | ||
} | ||
|
||
const client = new WebClient(BOT_TOKEN, options); |
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.
@seratch For the test script, I kept the bot token as the "main" token, and used the tooling token in an override manner, as I assume this is how most folks would use it in practice. Let me know if you think otherwise
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.
As long as both ways (1: passing in constructor 2: passing as argument) work, it's fine.
Summary
This pull request introduces support for the App Manifest endpoints, including:
apps.manifest.create
apps.manifest.delete
apps.manifest.export
apps.manifest.update
apps.manifest.validate
tooling.tokens.rotate
WebAPICallResult
should be switched out when the API responses become available and the generated types are updated.Requirements (place an
x
in each[ ]
)