Resource: service #219
Resource: service #219
Conversation
docs/resources/service_account.md
Outdated
| page_title: "twingate_service_account Resource - terraform-provider-twingate" | ||
| subcategory: "" | ||
| description: |- | ||
| Services offer a way to provide programmatic, centrally-controlled, and consistent access controls. For more information, see Twingate's documentation https://www.twingate.com/docs/services. |
There was a problem hiding this comment.
I know this is taken from the public docs, but let's do this everywhere: s/Services/Service Accounts/g
In this description, let's remove the sentence "For more information [...]"
There was a problem hiding this comment.
@alexmensch I've updated doc strings
| @@ -0,0 +1,84 @@ | |||
| package client | |||
There was a problem hiding this comment.
Why is this here and not in twingate/internal/test/client ?
There was a problem hiding this comment.
here I'm testing private func fetchPages and it's not accessible from other packages
| @@ -230,3 +230,31 @@ func (r ResourceNode) ToModel() *model.Resource { | |||
| func (q readResourcesByNameQuery) ToModel() []*model.Resource { | |||
| return q.Resources.ToModel() | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
Its something I missed in the restructure review but Im not sure I understand the purpose of converter.go - we define createServiceAccountQuery in service-account.go why cant its methods be there?
(same for all the other structs that their methods are here)
There was a problem hiding this comment.
or maybe the struct defintion should be here too?
or maybe we need queries/createServiceAccountQuery.go ?
There was a problem hiding this comment.
here in converter.go I've collected simple utilities functions which converts queries objects to models
I think in service-account.go would be better to have only code related to API request, and this object conversions like lower layer details
I like the idea with separate package for queries, I think this can help organise code better :)
There was a problem hiding this comment.
ok so lets merge this and refactor to queries on a separate PR
Resolves #67
Changes