feat: Add support for team memberships. - #95
Conversation
|
Requires #88. |
d881e92 to
53a6408
Compare
|
Hi @kad-hollac1 - thank you very much for this PR! I had made some changes to your previous contribution, in order to make the Also, your tests seem to be failing... |
96a8e2e to
7c25a0c
Compare
|
Thanks I've changed some, I hope it is good enough now, let me know otherwise. Sorry I pushed the ,,request review'' button too soon. ^^ |
acch
left a comment
There was a problem hiding this comment.
Thanks much for this @kad-hollac1! I do have three requests, though:
-
Let's call the resource and data source
forgejo_team_member. This is more consistent with the other resources, such asforgejo_collaborator. -
Let's reference users by their name instead of their ID. As users are frequently created outside of Terraform, referencing them by their name is more convenient. You can refer to the
forgejo_collaboratorschema for an example. -
Could you please add usage examples for
forgejo_teamandforgejo_team_memberto theexamples/folder? These are included in the documentation when markdown is rendered.
Funnily enough, we'll be using it mainly for bot-users that our developers create themselves. ;) I'd actually say we should still reference by ID, for consistency, and let practitioners use a |
b49c469 to
df83b39
Compare
d4eb80c to
d2952fd
Compare
|
Thanks much for these changes @kad-hollac1, the code looks very well now! However, the current convention is to reference organisations and repositories by their ID, and users by their name (for the reasons stated). In order to keep this consistent across the provider, we would either need to change all resources and data sources to reference users by ID ( |
|
Sorry, I totally missed that the convention for users is different than for other entities. It is your call. If you actually want the former, I can create a separate patch for using IDs in other resources. Else I'll update this to use usernames. |
|
Thanks @kad-hollac1. I'd prefer to go with the current convention and update this resource and data source to use the user's name. The other alternative would be a major breaking change, which I'd like to avoid... Thanks much for your continued efforts! |
d2952fd to
da6502a
Compare
da6502a to
5491aff
Compare
|
Thanks for the feedback, @acch. I've updated this merge request to use usernames. |
|
Thanks much for this @kad-hollac1! The code looks great, merging it now... |
|
Thanks for the review and merge! :) |
Fixes: #94.