-
Notifications
You must be signed in to change notification settings - Fork 99
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
build: bump go version #715
Conversation
Signed-off-by: dextrot <[email protected]>
Signed-off-by: dextrot <[email protected]>
Signed-off-by: dextrot <[email protected]>
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #715 +/- ##
==========================================
- Coverage 75.92% 75.62% -0.30%
==========================================
Files 59 64 +5
Lines 5873 5994 +121
==========================================
+ Hits 4459 4533 +74
- Misses 1037 1078 +41
- Partials 377 383 +6 ☔ View full report in Codecov by Sentry. |
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.
@dextrot The build failed because go 1.22 changed the implementation of io.NopCloser
.
To fix it, you need to update the code
Lines 47 to 58 in be59736
// nopCloserType is the type of `io.NopCloser()`. | |
var nopCloserType = reflect.TypeOf(io.NopCloser(nil)) | |
// UnwrapNopCloser unwraps the reader wrapped by `io.NopCloser()`. | |
// Similar implementation can be found in the built-in package `net/http`. | |
// Reference: https://github.com/golang/go/blob/go1.17.6/src/net/http/transfer.go#L423-L425 | |
func UnwrapNopCloser(rc io.Reader) io.Reader { | |
if reflect.TypeOf(rc) == nopCloserType { | |
return reflect.ValueOf(rc).Field(0).Interface().(io.Reader) | |
} | |
return rc | |
} |
to be updated with https://github.com/golang/go/blob/go1.22.1/src/net/http/transfer.go#L1090-L1105
More unit test cases should also be added to TestUnwrapNopCloser
.
@dextrot Since we need this PR in the incoming |
Signed-off-by: SKM <[email protected]>
Signed-off-by: SKM <[email protected]>
Done. |
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. Thanks for contribution!
Add a new unit test case by the change to `UnwrapNopCloser` introduced by #715 --------- Signed-off-by: Shiwei Zhang <[email protected]>
This PR fixes #713
README.md
,go.mod
andGitHub Actions
.