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

feat(team_membership): allow id lookup from slug #1502

Merged

Conversation

jhoenzsch
Copy link
Contributor

@jhoenzsch jhoenzsch commented Jan 20, 2023

Resolves #1501


Behavior

Before the change?

  • github_team_membership allows passing in the team slug as a team id.

After the change?

  • github_team_membership allows passing in the team slug as a team id.

Additional info

Pull request checklist

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)
  • Added the appropriate label for the given change

Does this introduce a breaking change?

Please see our docs on breaking changes to help!

  • Yes (Please add the Type: Breaking change label)
  • No

If Yes, what's the impact:

  • N/A

Pull request type

Please add the corresponding label for change this PR introduces:

  • Bugfix: Type: Bug
  • Feature/model/API additions: Type: Feature
  • Updates to docs or samples: Type: Documentation
  • Dependencies/code cleanup: Type: Maintenance

@jhoenzsch jhoenzsch force-pushed the feature/github-team-membership branch from e121a03 to 3f3dfb9 Compare January 20, 2023 22:41
@kfcampbell kfcampbell merged commit fdb2e0c into integrations:main Feb 3, 2023
@csainty
Copy link
Contributor

csainty commented Feb 5, 2023

This turned a parseInt() in to an api call - https://github.com/integrations/terraform-provider-github/blob/main/github/util.go#L163
We don't use this resource any longer, so I don't have a ready test setup, but unless there is caching down the stack this would have been an explosion of requests (X users * Y repos).

@dennislapchenko
Copy link
Contributor

dennislapchenko commented Feb 9, 2023

@jhoenzsch Hey. You think same can be done for github_team_members resource?
edit: did it myself 😁

@kfcampbell
Copy link
Member

@csainty are you experiencing this effect yourself yet?

@csainty
Copy link
Contributor

csainty commented Feb 13, 2023

@kfcampbell No we don't use it, I was just browsing the release notes and reading diffs.

reedloden pushed a commit to reedloden/terraform-provider-github that referenced this pull request Jun 14, 2023
* feat(team_membership): allow id lookup from slug

* feat(team_membership): allow id lookup from slug

* Run linter

---------

Co-authored-by: Jo Hoenzsch <jo.hoenzsch@sentinelone.com>
Co-authored-by: Keegan Campbell <me@kfcampbell.com>
avidspartan1 pushed a commit to avidspartan1/terraform-provider-github that referenced this pull request Feb 5, 2024
* feat(team_membership): allow id lookup from slug

* feat(team_membership): allow id lookup from slug

* Run linter

---------

Co-authored-by: Jo Hoenzsch <jo.hoenzsch@sentinelone.com>
Co-authored-by: Keegan Campbell <me@kfcampbell.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEAT]: github_team_membership doesn't allow slug instead of team id
5 participants