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

[good-first] Deprecate hashset #3091

Closed
RainbowMango opened this issue Feb 1, 2023 · 4 comments · Fixed by #3115
Closed

[good-first] Deprecate hashset #3091

RainbowMango opened this issue Feb 1, 2023 · 4 comments · Fixed by #3115
Assignees
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines.

Comments

@RainbowMango
Copy link
Member

Task description:
After update Kubernetes dependencies to v1.26(#3080), we are able to use the generic set. So the previous hashset , introduced by #2958, cloud be deprecated and removed from the code base.

Solution:

  1. Replace hashset with sets.Set:
diff --git a/pkg/resourceinterpreter/configurableinterpreter/configurable.go b/pkg/resourceinterpreter/configurableinterpreter/configurable.go
index 032fe7f6..131f957e 100644
--- a/pkg/resourceinterpreter/configurableinterpreter/configurable.go
+++ b/pkg/resourceinterpreter/configurableinterpreter/configurable.go
@@ -6,6 +6,7 @@ import (
        "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
        "k8s.io/apimachinery/pkg/runtime"
        "k8s.io/apimachinery/pkg/runtime/schema"
+       "k8s.io/apimachinery/pkg/util/sets"
        "k8s.io/klog/v2"

        configv1alpha1 "github.com/karmada-io/karmada/pkg/apis/config/v1alpha1"
@@ -13,7 +14,6 @@ import (
        "github.com/karmada-io/karmada/pkg/resourceinterpreter/configurableinterpreter/configmanager"
        "github.com/karmada-io/karmada/pkg/resourceinterpreter/configurableinterpreter/luavm"
        "github.com/karmada-io/karmada/pkg/util/fedinformer/genericmanager"
-       "github.com/karmada-io/karmada/pkg/util/hashset"
 )

 // ConfigurableInterpreter interprets resources with resource interpreter customizations.
@@ -153,7 +153,7 @@ func (c *ConfigurableInterpreter) GetDependencies(object *unstructured.Unstructu
                return
        }

-       refs := hashset.Make[configv1alpha1.DependentObjectReference]()
+       refs := sets.New[configv1alpha1.DependentObjectReference]()
        for _, luaScript := range scripts {
                var references []configv1alpha1.DependentObjectReference
                references, err = c.luaVM.GetDependencies(object, luaScript)
@@ -164,7 +164,7 @@ func (c *ConfigurableInterpreter) GetDependencies(object *unstructured.Unstructu
                }
                refs.Insert(references...)
        }
-       dependencies = refs.List()
+       dependencies = refs.UnsortedList()

        // keep returned items in the same order between each call.
        sort.Slice(dependencies, func(i, j int) bool {
  1. Remove hashset from Karmada codebase.

Who can join or take the task:

The good first issue is intended for first-time contributors to get started on his/her contributor journey.

After a contributor has successfully completed 1-2 good first issue's,
they should be ready to move on to help wanted items, saving the remaining good first issue for other new contributors.

How to join or take the task:

Just reply on the issue with the message /assign in a separate line.

Then, the issue will be assigned to you.

How to ask for help:

If you need help or have questions, please feel free to ask on this issue.
The issue author or other members of the community will guide you through the contribution process.

@RainbowMango RainbowMango added the good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. label Feb 1, 2023
@zzhdao
Copy link

zzhdao commented Feb 1, 2023

/assign

@RainbowMango
Copy link
Member Author

Hi @zzhdao Any update?
I'm not trying to push you, just because I see @VedRatan sent a PR(#3103) for this.
Let me know if you need any help.

@VedRatan
Copy link
Contributor

VedRatan commented Feb 6, 2023

I'm sorry but this issue is been opened since a long time so I tried to resolve it hope @zzhdao will not mind and also @RainbowMango

@RainbowMango
Copy link
Member Author

@zzhdao I'm going to re-assign this task to @VedRatan, I assume you are not available recently. Hope you don't mind.
Thanks all the same though.

/unassign @zzhdao
/assign @VedRatan

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants