-
Notifications
You must be signed in to change notification settings - Fork 104
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
Adding funk library to the project #1237
Conversation
which implements map, filter etc. methods for slices and maps. First commit replaces existing implementations of the `contains` method through the code base.
@@ -528,7 +519,7 @@ func remove(values []string, s string) (result []string) { | |||
// hasn't been added yet, the instance has a cleanup plan and the cleanup plan | |||
// didn't run yet. Returns true if the cleanup finalizer has been added. | |||
func (i *Instance) TryAddFinalizer() bool { | |||
if !contains(i.ObjectMeta.Finalizers, instanceCleanupFinalizerName) { | |||
if !funk.ContainsString(i.ObjectMeta.Finalizers, instanceCleanupFinalizerName) { |
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.
Typed methods, e.g. ContainsString, ContainsInt
do not use reflection and have the same performance as the replaced implementation.
CreateFunc: func(event.CreateEvent) bool { return true }, | ||
DeleteFunc: func(e event.DeleteEvent) bool { | ||
return e.Meta.GetAnnotations() != nil && | ||
funk.Contains(e.Meta.GetAnnotations(), task.PipePodAnnotation) |
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.
Using the untyped method Contains
since annotations is a map[string]string
and has no typed implementation.
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.
@zen-dog catching up, so is this using reflect
then?
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.
Welcome back, @gerred ;) yes, the generic methods e.g. Contains
use reflection to detect the correct type. It is yet to be proved, but I suspect that the majority of our use cases could be covered by the typed methods e.g. ContainsString
.
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.
I curious regarding selection criteria... other options are Koazee, go-linq.
according to https://medium.com/wesovilabs/koazee-vs-go-funk-vs-go-linq-caf8ef18584e koazee is more performant.
looking at projects: koazee and funk seem to have the same level of project activity and both have a small number of open issues. It looks like koazee is better organized (packages).
I'm for this or similar library!
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.
This is great, also good to know that funk
provides typesafe implementations for common types.
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.
🎉 for removing code!
@@ -8,7 +8,6 @@ require ( | |||
github.com/Masterminds/sprig v2.22.0+incompatible | |||
github.com/Microsoft/go-winio v0.4.14 // indirect | |||
github.com/containerd/containerd v1.2.9 // indirect | |||
github.com/coreos/etcd v3.3.15+incompatible |
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.
This seems unrelated?
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.
I ran go mod tidy
so probably some remnant ¯_(ツ)_/¯
which implements map, filter, etc. methods for slices and maps. The first commit replaces existing implementations of the `contains` method through the codebase. Signed-off-by: Andreas Neumann <aneumann@mesosphere.com>
which implements map, filter, etc. methods for slices and maps. The first commit replaces existing implementations of the
contains
method through the codebase.