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

timeafter: skip check on Go ≥ 1.23 #48

Merged
merged 1 commit into from
Oct 30, 2024

Conversation

tklauser
Copy link
Member

@tklauser tklauser commented Oct 29, 2024

Go version ≥ 1.23 no longer has the issue of not collecting unstopped Timers and time.After can safely be used in loops. Also see https://go.dev/doc/go1.23#timer-changes and
https://cs.opensource.google/go/go/+/refs/tags/go1.23.2:src/time/sleep.go;l=196-201

Thus we can skip the timeafter check on modules with go.mod specifying go1.23 or later.

Example run:

$ mkdir timeafter && cd timeafter
$ cat <<EOF > main.go
package main

import "time"

func main() {
	for {
		select {
		case <-time.After(5 * time.Second):
		}
	}
}
$ cat <<EOF > go.mod
module timeafter

go 1.22.0
EOF
$ linters .
/home/tklauser/tmp/timeafter/main.go:8:10: use of time.After in a for loop is prohibited, use inctimer instead
$ sed -ie 's/go 1\.22\.0/go 1.23.0/' go.mod
$ linters .
$

@tklauser tklauser force-pushed the pr/tklauser/go-1.23-skip-inctimer branch 2 times, most recently from 61aa912 to 26d0bd9 Compare October 30, 2024 10:18
Go version ≥ 1.23 no longer has the issue of not collecting unstopped
Timers and time.After can safely be used in loops. Also see
https://go.dev/doc/go1.23#timer-changes and
https://cs.opensource.google/go/go/+/refs/tags/go1.23.2:src/time/sleep.go;l=196-201

Thus we can skip the timeafter check on modules with go.mod specifying
go1.23 or later.

Example run:

```
$ mkdir timeafter && cd timeafter
$ cat <<EOF > main.go
package main

import "time"

func main() {
	for {
		select {
		case <-time.After(5 * time.Second):
		}
	}
}
$ cat <<EOF > go.mod
module timeafter

go 1.22.0
EOF
$ linters .
/home/tklauser/tmp/timeafter/main.go:8:10: use of time.After in a for loop is prohibited, use inctimer instead
$ sed -ie 's/go 1\.22\.0/go 1.23.0/' go.mod
$ linters .
$
```

Signed-off-by: Tobias Klauser <[email protected]>
@tklauser tklauser force-pushed the pr/tklauser/go-1.23-skip-inctimer branch from 26d0bd9 to 841828c Compare October 30, 2024 10:23
@tklauser tklauser requested a review from rolinh October 30, 2024 10:23
Copy link
Member

@rolinh rolinh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good one 🎉

@rolinh rolinh merged commit 32a1a70 into main Oct 30, 2024
10 checks passed
@rolinh rolinh deleted the pr/tklauser/go-1.23-skip-inctimer branch October 30, 2024 12:44
tklauser added a commit to cilium/cilium that referenced this pull request Oct 30, 2024
This is the first tagged version and includes support for skipping the
timeafter check on Go version ≥ 1.23 [1] which is a prerequisite to
dropping the inctimer package.

[1] cilium/linters#48

Signed-off-by: Tobias Klauser <[email protected]>
tklauser added a commit to cilium/cilium that referenced this pull request Oct 31, 2024
This is the first tagged version and includes support for skipping the
timeafter check on Go version ≥ 1.23 [1] which is a prerequisite to
dropping the inctimer package.

[1] cilium/linters#48

Signed-off-by: Tobias Klauser <[email protected]>
tklauser added a commit to cilium/cilium that referenced this pull request Oct 31, 2024
This is the first tagged version and includes support for skipping the
timeafter check on Go version ≥ 1.23 [1] which is a prerequisite to
dropping the inctimer package.

[1] cilium/linters#48

Signed-off-by: Tobias Klauser <[email protected]>
github-merge-queue bot pushed a commit to cilium/cilium that referenced this pull request Oct 31, 2024
This is the first tagged version and includes support for skipping the
timeafter check on Go version ≥ 1.23 [1] which is a prerequisite to
dropping the inctimer package.

[1] cilium/linters#48

Signed-off-by: Tobias Klauser <[email protected]>
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