Skip to content
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

Merged
merged 1 commit into from
Jul 11, 2024
Merged

Conversation

displague
Copy link
Member

@displague displague commented May 22, 2024

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

@displague displague requested a review from a team as a code owner May 22, 2024 16:12
}
}

// Subsets returns true if the first slice is a subset of the second slice
Copy link
Member Author

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:

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
Copy link
Member Author

@displague displague May 22, 2024

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.

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
}

equinix/provider.go Outdated Show resolved Hide resolved
@@ -0,0 +1,89 @@
package comparisons_test
Copy link
Member Author

@displague displague Jun 4, 2024

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]>
@ctreatma ctreatma merged commit 54b4ae7 into main Jul 11, 2024
5 of 6 checks passed
@ctreatma ctreatma deleted the comparisons branch July 11, 2024 18:57
Copy link

This PR is included in version 2.2.0 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants