-
Notifications
You must be signed in to change notification settings - Fork 676
feat(web-api): add workflows.featured.{add|list|remove|set} methods #2303
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
Conversation
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #2303 +/- ##
==========================================
+ Coverage 92.69% 92.74% +0.05%
==========================================
Files 38 38
Lines 10609 10660 +51
Branches 683 687 +4
==========================================
+ Hits 9834 9887 +53
+ Misses 763 761 -2
Partials 12 12
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
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.
📝 Leaving a few notes for the most kind reviewers. FYI I'm hoping to make similar changes to other SDKs soon as well-
packages/web-api/src/types/response/WorkflowsFeaturedListResponse.ts
Outdated
Show resolved
Hide resolved
@@ -964,7 +964,7 @@ function parseRetryHeaders(response: AxiosResponse): number | undefined { | |||
* @param logger instance of web clients logger | |||
*/ | |||
function warnDeprecations(method: string, logger: Logger): void { | |||
const deprecatedMethods = ['workflows.']; | |||
const deprecatedMethods = ['workflows.stepCompleted', 'workflows.stepFailed', 'workflows.updateStep']; |
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.
📣 Somewhat related change to avoid warnings of these new methods being deprecated:
[WARN] web-api:WebClient:0 workflows.featured.add is deprecated. Please check on https://docs.slack.dev/reference/methods for an alternative.
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.
Nice; did ['workflows.']
trigger this warning for any method that started with workflows
?
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 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 :)
Only thought is that the PR could potentially add some tests
Co-authored-by: Elaine Vegeris <[email protected]>
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.
Thanks for working on this 💯
@vegeris @WilliamBergamin Thank y'all both for the review and of course the needed tests! I'm going to merge this now but will follow up soon with the related PRs in other SDKs. Perhaps soon too we can cut a release with these new methods? 🚢 💨 |
Summary
This PR adds the
workflows.featured.{add|list|remove|set}
methods:Reviewers
Gather shortcut trigger IDs and a user token for the following methods:
Requirements