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

can remove funcr to an independent repo? #132

Closed
soluty opened this issue Jan 25, 2022 · 8 comments
Closed

can remove funcr to an independent repo? #132

soluty opened this issue Jan 25, 2022 · 8 comments
Assignees

Comments

@soluty
Copy link

soluty commented Jan 25, 2022

thanks to nice work, because of the funcr package use a api strconv.FormatComplex that come from go1.16, if some one want to use this log interface, he must update his golang version, this seems no need for a minimal log interface

@pohly
Copy link
Contributor

pohly commented Jan 25, 2022

Are you asking because of "go 1.16" in the go.mod file or because there are some actual problems?

I could compile logr and (surprisingly) also funcr with 1.15.6. It was only 1.14.4 where funcr (and only funcr) failed:

logr$ /nvme/gopath/go-1.14.4/bin/go build .
logr$ /nvme/gopath/go-1.14.4/bin/go build ./funcr
# github.com/go-logr/logr/funcr
funcr/funcr.go:398:16: undefined: strconv.FormatComplex
funcr/funcr.go:400:16: undefined: strconv.FormatComplex
funcr/funcr.go:442:16: undefined: strconv.FormatComplex
funcr/funcr.go:444:16: undefined: strconv.FormatComplex
note: module requires Go 1.16

I think "go 1.16" is just providing some hints about how the code is to be compiled. It's not treated as a hard requirement.

@thockin
Copy link
Contributor

thockin commented Jan 26, 2022

https://go.dev/doc/go1.15

https://pkg.go.dev/strconv#FormatComplex

So we could lower the module requirement if that is the thing causing pain. I did some digging and it's possible to have build tags conditional on go version, so we could do:

funcr/go_lt_1_15.go:

//go:build !go1.15
// +build !go1.15

//  Only used on Go versions less than v1.15
func formatComplex() { /* use Sprintf(%v) */ }

and

funcr/go_ge_1_15.go:

//go:build go1.15
// +build go1.15

// Only used on Go versions v1.15 or higher.
func formatComplex() { /* use FormatComplex() */ }

PR welcome, I think. @pohly - would you be OK with that?

@thockin thockin self-assigned this Jan 26, 2022
@pohly
Copy link
Contributor

pohly commented Jan 26, 2022

https://go.dev/doc/go1.15

https://pkg.go.dev/strconv#FormatComplex

So FormatComplex was added in 1.15, not 1.16 as the issue description says.

So we could lower the module requirement if that is the thing causing pain.

I'm not sure whether it really does. It's not a hard requirement and Go 1.15 works just fine as it is. Let's wait for a response from @soluty with a clarification.

Regarding Go < 1.15: those are out of support by the Go team already. I don't think we should be doing anything special for them unless we have a good reason. Just my two cents.

@pohly
Copy link
Contributor

pohly commented Jan 27, 2022

Besides clarifying which Go version we support we also need to fix testing with those: we have a GitHub action which iterates over 1.14, 1.15, 1.16, but then always installs 1.15...

@thockin
Copy link
Contributor

thockin commented Jan 27, 2022

I just looked at the github action, and docs for it - it seems right, but clearly isn't...

@thockin
Copy link
Contributor

thockin commented Jan 27, 2022

Opened #134 and #135 to track actions and drop 1.14.

The question is really - should we keep 1.14 support?

@pohly
Copy link
Contributor

pohly commented Jan 27, 2022

The question is really - should we keep 1.14 support?

I don't think so.

@soluty, this is your chance to bring up arguments in favor of it....

@thockin
Copy link
Contributor

thockin commented Feb 12, 2022

I'm going to close this for now. If there are new argument, let's hear them.

@thockin thockin closed this as completed Feb 12, 2022
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

No branches or pull requests

3 participants