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

Upgrade to go v1.22 #4102

Closed
wants to merge 2 commits into from
Closed

Upgrade to go v1.22 #4102

wants to merge 2 commits into from

Conversation

scr-oath
Copy link
Contributor

@scr-oath scr-oath commented Dec 13, 2024

Copy link
Contributor

@SyntaxNode SyntaxNode left a comment

Choose a reason for hiding this comment

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

There are also references to the Go version in:

  • \Dockerfile
  • .github\workflows\adapter-code-coverage.yaml
  • .github\workflows\validate-merge.yaml
  • .github\workflows\validate.yaml

go.mod Outdated
@@ -1,6 +1,6 @@
module github.com/prebid/prebid-server/v3

go 1.21
go 1.23
Copy link
Contributor

Choose a reason for hiding this comment

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

We target to support the previous two Go releases. Therefore, please bump this up to only version 1.22.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How is the "supporting two releases" done by choosing an older version? Do you mean, instead, that you always stay on the "previous release"?

Copy link
Contributor Author

@scr-oath scr-oath Dec 13, 2024

Choose a reason for hiding this comment

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

Not setting to 1.23 means that we can't start using the iter package yet, which can make some utilities that improve walking, say… []Bid (by walking the pointers to the Bid rather than making a copy of them for each iteration.

Copy link
Contributor Author

@scr-oath scr-oath Dec 13, 2024

Choose a reason for hiding this comment

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

Here's an example file we can add to an iterators module in util

package iterators

import "iter"

// SlicePointers returns an iterator that yields indices and pointers to the elements of a slice.
func SlicePointers[Slice ~[]T, T any](s Slice) iter.Seq2[int, *T] {
	return func(yield func(int, *T) bool) {
		for i := range s {
			if !yield(i, &s[i]) {
				break
			}
		}
	}
}

// SlicePointerValues returns an iterator that yields pointers to the elements of a slice.
func SlicePointerValues[Slice ~[]T, T any](s Slice) iter.Seq[*T] {
	return func(yield func(*T) bool) {
		for i := range s {
			if !yield(&s[i]) {
				break
			}
		}
	}
}

And then use a la

		for seatBid := range iterators.SlicePointerValues(payload.BidResponse.SeatBid) {
			for bid := range iterators.SlicePointerValues(seatBid.Bid) {

Copy link
Contributor

Choose a reason for hiding this comment

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

How is the "supporting two releases" done by choosing an older version? Do you mean, instead, that you always stay on the "previous release"?

We build the Docker image using the latest version to take advantage of compiler / runtime improvements and restrict language features to the previous version.

Not setting to 1.23 means that we can't start using the iter package yet, which can make some utilities that improve walking, say… []Bid (by walking the pointers to the Bid rather than making a copy of them for each iteration.

Correct. We'll wait a little bit for those new features.

@scr-oath scr-oath changed the title Upgrade to go v1.23 Upgrade to go v1.22 Dec 13, 2024
@bsardo
Copy link
Collaborator

bsardo commented Dec 16, 2024

Discussed offline with @SyntaxNode. Closing as we have since opened #4112.

@bsardo bsardo closed this Dec 16, 2024
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.

3 participants