INTMDB-128: Modified design when you can get .id from various resources #471
Conversation
…cit depedency with id from various resources instead of another id like container_id
themantissa
requested review from
a team and
threebee
and removed request for
a team
|
Okay adding the explanation here. |
There was a problem hiding this comment.
Hi @coderGo93 awesome work, just one request could we create a terratest for all the modified resources, just for the sake of sanity.
And in addition to this could we add a datasource for project , and re-use the id of that datasource in the other resources.
| ImportState: true, | ||
| ImportStateVerify: true, | ||
| ResourceName: resourceName, | ||
| ImportStateIdFunc: testAccCheckMongoDBAtlasPrivateLinkEndpointServiceImportStateIDFunc(resourceName), |
There was a problem hiding this comment.
I have a question, is it necessary to use so long func names?
I mean, in this method you are in mongodbatlas_privatelink_endpoint...method, maybe the name for this func can be testAccCheckServiceImportStateIDFunc
having names like this can affect the readability and the maintainability
There was a problem hiding this comment.
As long it doesn't cross the limit of the line-length from golangci.yml it should be fine, mm it seems readable for me since it's usually testAccCheckMongoDBAtlas[Resource/datasource][Extra(like ImportState, destroy, etc] something like that, but it somehow it crosses the limit, yes it would be nice to shorten it and still readable for the resource it belongs to
Done, thank you for the suggestion @leofigy |
container_idfrom various resourceDescription
Please include a summary of the fix/feature/change, including any relevant motivation and context.
Link to any related issue(s):
Type of change:
Required Checklist:
Further comments
#320