search

metal-stack / metal-lib

Common go packages used across the metal-stack microservices
MIT License
2 stars 1 forks source link

Adopt to breaking change in x/exp #96

Closed majst01 closed 1 year ago

majst01 commented 1 year ago

@vknabel Gerrit and me struggled to find the correct int values to return, maybe you can have a extra look

The Problem is bigger:

go test -coverprofile cover.out -cover -race ./... && go tool cover -func cover.out
?       github.com/metal-stack/metal-lib        [no test files]
?       github.com/metal-stack/metal-lib/auditing       [no test files]
?       github.com/metal-stack/metal-lib/bus/testenv    [no test files]
ok      github.com/metal-stack/metal-lib/auth   0.910s  coverage: 37.1% of statements
# tailscale.com/net/tsaddr
../../../../go/pkg/mod/tailscale.com@v1.46.0/net/tsaddr/tsaddr.go:255:21: type func(ri netip.Prefix, rj netip.Prefix) bool of func(ri, rj netip.Prefix) bool {…} does not match inferred type func(a netip.Prefix, b netip.Prefix) int for func(a E, b E) int

@vknabel same problem with the meilisearch.go :-(

majst01 commented 1 year ago

This was caused by: https://github.com/golang/exp/commit/302865e7556b4ae5de27248ce625d443ef4ad3ed

Maybe we should wait for go-1.21 (comming Sept.23) and use the slices package from there and keep x/exp behind

Current impl in go master: https://github.com/golang/go/blob/master/src/slices/sort.go#L35

@Gerrit91 @vknabel WDYT ?

vknabel commented 1 year ago

Fixing this in our code is relatively easy, but as there are 3rd party dependencies involved (tailscale), I'd prefer to wait. Especially as it's not a critical component. Later we just replace golang.org/x/exp/slices with probably slices, apply the file changes of this PR and are probably done (no matter if tailscale does).

majst01 commented 1 year ago

Decision was made to postpone this until go-1.21 was released, then we can switch to the standard library slices.

I will keep this PR open for reference

vknabel commented 1 year ago

I just upgraded metal-lib to Go 1.21 and used the recently released slices standard lib package. Ready for review by @Gerrit91 (@majst01 and I did most code here)

majst01 commented 1 year ago

We should also check for other occurrences of golang.org/x/exp and remove them now if possible

majst01 commented 1 year ago

We should also check for other occurrences of golang.org/x/exp and remove them now if possible

https://github.com/metal-stack/metal-lib/pull/101