-
Notifications
You must be signed in to change notification settings - Fork 833
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
feat: add team.externalTeams.list #1512
feat: add team.externalTeams.list #1512
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1512 +/- ##
==========================================
- Coverage 85.04% 84.88% -0.16%
==========================================
Files 112 112
Lines 12440 12473 +33
==========================================
+ Hits 10579 10588 +9
- Misses 1861 1885 +24 ☔ View full report in Codecov by Sentry. |
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 but let's hold off on merging until the API goes public.
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 it! It seems the endpoint is not yet public. Let's hold off merging this PR until the API becomes publicly available.
@@ -17,7 +17,7 @@ | |||
class TestWebClientCoverage(unittest.TestCase): | |||
# 284 endpoints as of June 12, 2024 | |||
# Can be fetched by running `var methodNames = [].slice.call(document.getElementsByClassName('apiReferenceFilterableList__listItemLink')).map(e => e.href.replace("https://api.slack.com/methods/", ""));console.log(methodNames.toString());console.log(methodNames.length);` on https://api.slack.com/methods |
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 list of methods is the output by this JS script. "team.externalTeams.list" is not yet public, so let's hold off making the manual change here until it becomes public.
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.
Sounds good 👍
Co-authored-by: Kazuhiro Sera <[email protected]>
Co-authored-by: Kazuhiro Sera <[email protected]>
Co-authored-by: Kazuhiro Sera <[email protected]>
@WilliamBergamin this API is now live, so when ready, we should merge and release. |
If you have the bandwidth, adding an integration tests for this API call is a great addition for safety and future maintenance (but it's optional) |
Summary
This PR implements the team.externalTeams.list method for the python clients
Category (place an
x
in each of the[ ]
)/docs-src
(Documents, have you run./scripts/docs.sh
?)/docs-src-v2
(Documents, have you run./scripts/docs-v2.sh
?)/tutorial
(PythOnBoardingBot tutorial)tests
/integration_tests
(Automated tests for this library)Requirements (place an
x
in each[ ]
)python3 -m venv .venv && source .venv/bin/activate && ./scripts/run_validation.sh
after making the changes.