-
Notifications
You must be signed in to change notification settings - Fork 47
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
refactor: add comparisons pkg for reused helpers #682
Conversation
} | ||
} | ||
|
||
// Subsets returns true if the first slice is a subset of the second slice |
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.
Compared to the original, this function uses generics:
terraform-provider-equinix/equinix/provider.go
Lines 197 to 204 in 3cfe64e
func stringsFound(source []string, target []string) bool { | |
for i := range source { | |
if !slices.Contains(target, source[i]) { | |
return false | |
} | |
} | |
return true | |
} |
return true | ||
} | ||
|
||
// comparisons.SlicesMatch returns true if the two slices contain the same elements, regardless of order |
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.
compared to the original, this function uses slices
, is more efficient (in ways we won't hit in this project), and is easier to read in my opinion.
terraform-provider-equinix/equinix/provider.go
Lines 232 to 254 in 3cfe64e
func slicesMatch(s1, s2 []string) bool { | |
if len(s1) != len(s2) { | |
return false | |
} | |
visited := make([]bool, len(s1)) | |
for i := 0; i < len(s1); i++ { | |
found := false | |
for j := 0; j < len(s2); j++ { | |
if visited[j] { | |
continue | |
} | |
if s1[i] == s2[j] { | |
visited[j] = true | |
found = true | |
break | |
} | |
} | |
if !found { | |
return false | |
} | |
} | |
return true | |
} |
@@ -0,0 +1,89 @@ | |||
package comparisons_test |
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.
Tests were taken from https://github.com/equinix/terraform-provider-equinix/blob/comparisons/equinix/provider_test.go#L147-L254
Those tests can be removed with the deprecated functions they test.
This commit moves the helper functions from provider.go to a new comparisons package. This package will be used by provider, tests, and resources. The refactoring will assist with splitting resources into resource specific packages. The previous helper functions have been deprecated and will be removed in a future commit. Signed-off-by: Marques Johansson <[email protected]>
This PR is included in version 2.2.0 🎉 |
This PR moves the helper functions from provider.go to a new comparisons package. This package will be used by provider, tests, and resources. The refactoring will assist with splitting resources into resource specific packages. #106
The previous helper functions have been deprecated and will be removed in a future commit.
Extracted from #622 (PR grew to large to maintain. Refactoring is not a priority and rebasing gets hard as the underlying code changes)
Related to #674
Related to #654