resource/account_member: Use TypeSet, not TypeList
#876
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When I created this resource, `TypeList` seemed to be the correct choice for the data type we store. However, after a while we discovered an issue whereby a change in the ordering of the `role_ids`, resulted in the account member wanting to be recreated. This was addressed in #128 however it meant that new roles had to be added to the end otherwise it would flap. It also had the extra legwork of a custom `DiffSuppressFunc` to ensure that we compared the values using a consistent ordering to avoid the trap where the value wasn't added to the end. Now that we understand the internal types a little better, `TypeSet` is a more appropriate choice as it is an unordered list which fits the `role_ids` perfectly and solves the initial issues we encountered.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When I created this resource,
TypeListseemed to be the correct choicefor the data type we store. However, after a while we discovered an
issue whereby a change in the ordering of the
role_ids, resulted in theaccount member wanting to be recreated. This was addressed in #128
however it meant that new roles had to be added to the end otherwise it
would flap. It also had the extra legwork of a custom
DiffSuppressFuncto ensure that we compared the values using a consistent ordering to
avoid the trap where the value wasn't added to the end.
Now that we understand the internal types a little better,
TypeSetisa more appropriate choice as it is an unordered list which fits the
role_idsperfectly and solves the initial issues we encountered.