-
-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
chore: Bump up to Go 1.19, minimum 1.18 #4925
Conversation
Runs `gofmt -w -r 'interface{} -> any' .` This command made unnecessary changes to code comments though, so I reverted those by hand.
Before merging, we'll need to update the required CI tests (add 1.19, remove 1.17) |
Can we make use of this at all to make |
We'll probably want to adjust things to use the new |
We'll probably want to mention this in the release notes: https://tip.golang.org/doc/go1.18#sha1 |
We can probably update a lot of |
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.
Whew, I reviewed every changed line in this PR!
Before merging, we'll need to update the required CI tests (add 1.19, remove 1.17)
Done. Well, I removed 1.17, can't add 1.19 until this is merged I think.
Can we make use of this at all to make ./caddy version suck less for development go build builds? tip.golang.org/doc/go1.18#go-version
Yes, absolutely! Maybe a separate PR? I don't mind working on this one (unless you want to).
We'll probably want to adjust things to use the new netip.Addr type tip.golang.org/doc/go1.18#netip
Yeah, although that might be pretty invasive. Definitely a separate PR, and maybe after our next release (like after 2.6).
We'll probably want to mention this in the release notes: tip.golang.org/doc/go1.18#sha1
Good idea.
We can probably update a lot of strings.Index usages to use the new strings.Cut in many cases tip.golang.org/doc/go1.18#strings
That's quite likely, yeah.
Thanks for the quick PR!
Yeah my above notes are mostly TODO that should be made into separate issues probably. I don't think I'll work on those myself right now, just what I noticed from reading the 1.18 and 1.19 releases notes with my Caddy goggles on. |
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.
Looking good! 🎉 @mholt has already beat me to the details 😁
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.
LGTM, we should be good with RHEL 9.1.
cc: @carlwgeorge
No description provided.