-
Notifications
You must be signed in to change notification settings - Fork 40
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
FFM-9086: Add a terraform resource to create a new target group #661
Conversation
d5b6b99
to
5285755
Compare
* Add resource feature_flag_target_group * Add unit tests * Update documents and release note
f9c4efd
to
6760d1c
Compare
2f73a51
to
36b33e7
Compare
internal/service/platform/feature_flag_target_group/resource_feature_flag_target_group.go
Show resolved
Hide resolved
internal/service/platform/feature_flag_target_group/resource_feature_flag_target_group.go
Outdated
Show resolved
Hide resolved
internal/service/platform/feature_flag_target_group/resource_feature_flag_target_group.go
Outdated
Show resolved
Hide resolved
internal/service/platform/feature_flag_target_group/resource_feature_flag_target_group.go
Outdated
Show resolved
Hide resolved
internal/service/platform/feature_flag_target_group/resource_feature_flag_target_group.go
Outdated
Show resolved
Hide resolved
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.
Overall looks good.
There is some whitespace changes to files that can be reverted.
Take a look at the descriptions fro the resources, I think these need some changes.
One query over the sleep - do we need to use it, could we check the resp code from the create operation?
ff76f66
to
8578add
Compare
internal/service/platform/feature_flag_target_group/resource_feature_flag_target_group.go
Outdated
Show resolved
Hide resolved
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.
Please make one last change to the rules description.
The list of rules used to include targets in the target group
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.
generate docs
trigger build |
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.
Looks good
Describe your changes
Comment Triggers
PR Check triggers
trigger build
trigger subcategoryfieldcheck