-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Remove go 1.19 support, bump minimum to go 1.20 and add testing for 1.21 #8208
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #8208 +/- ##
=======================================
Coverage 90.27% 90.27%
=======================================
Files 301 301
Lines 15581 15581
=======================================
Hits 14065 14065
Misses 1227 1227
Partials 289 289
☔ View full report in Codecov by Sentry. |
There is a builder test consistently failing on this PR that is not failing on main
|
I think the error is related to actions/runner-images/issues/712, although I am not sure why this only happens with Go 1.20. |
I decided to skip the test and just list it on #5403 together with the other test that is already failing because of the same reason |
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.
To handle the required 1.19
check, I assume we remove the check, merge, and then add 1.21 check.
Co-authored-by: bryan-aguilar <46550959+bryan-aguilar@users.noreply.github.com>
@@ -133,7 +133,7 @@ jobs: | |||
unittest: | |||
strategy: | |||
matrix: | |||
go-version: ["1.20", 1.19] # 1.20 needs quotes otherwise it's interpreted as 1.2 | |||
go-version: ["1.21", "1.20"] # 1.20 needs quotes otherwise it's interpreted as 1.2 |
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.
Should this also use the ~
convention to force min version? Aligning across actions could make search and replace easier in the future.
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.
Happy to do that, but I am worried that ~
is not a supported character on cache keys and since we use this variable here
key: unittest-${{ runner.os }}-go-build-${{ matrix.go-version }}-${{ hashFiles('**/go.sum') }} |
Apparently, I cannot update the protected branch rules. Submitted open-telemetry/community#1636 |
This is blocked by #8218 |
|
To not update required jobs every time a new go version is released. Needed for #8208
Fixes #8207