Skip to content

build: bump go version#715

Merged
shizhMSFT merged 7 commits into
oras-project:mainfrom
dextrot:bump
Mar 19, 2024
Merged

build: bump go version#715
shizhMSFT merged 7 commits into
oras-project:mainfrom
dextrot:bump

Conversation

@dextrot

@dextrot dextrot commented Feb 27, 2024

Copy link
Copy Markdown
Contributor

This PR fixes #713

  • bumps go version in README.md, go.mod and GitHub Actions.

@dextrot
bump go.mod
73ea2b4 
Signed-off-by: dextrot <131444479+dextrot@users.noreply.github.com>
@dextrot
bump README.md
cfaff33 
Signed-off-by: dextrot <131444479+dextrot@users.noreply.github.com>
@dextrot
bump go version in GH Actions
2f56826 
Signed-off-by: dextrot <131444479+dextrot@users.noreply.github.com>
@shizhMSFT shizhMSFT changed the title Bump go version build: bump go version Mar 6, 2024
@github-advanced-security

github-advanced-security AI commented Mar 6, 2024

Copy link
Copy Markdown

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

codecov Bot commented Mar 6, 2024 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.62%. Comparing base (0285961) to head (30298d1).

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.
📢 Have feedback on the report? Share it here.

shizhMSFT
shizhMSFT requested changes Mar 6, 2024
View reviewed changes

@shizhMSFT shizhMSFT left a comment •
edited
Loading

Copy link
Copy Markdown
Contributor

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

oras-go/internal/ioutil/io.go

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.

@shizhMSFT shizhMSFT mentioned this pull request Mar 11, 2024
Closed
dextrot added 2 commits March 11, 2024 14:17
@dextrot
Merge branch 'main' into bump
3e12869
@dextrot
update io.NopCloser implementation acc to go 1.22
1ab83cd 
Signed-off-by: SKM <131444479+dextrot@users.noreply.github.com>
shizhMSFT
shizhMSFT requested changes Mar 11, 2024
View reviewed changes
Comment thread internal/ioutil/io.go Outdated
Comment thread internal/ioutil/io.go
@qweeah qweeah mentioned this pull request Mar 12, 2024
Merged
4 tasks
@shizhMSFT

shizhMSFT commented Mar 18, 2024

Copy link
Copy Markdown
Contributor

@dextrot Since we need this PR in the incoming v2.5.0 version, would you like to address the comments?

dextrot
dextrot commented Mar 18, 2024
View reviewed changes
Comment thread internal/ioutil/io.go
dextrot
dextrot commented Mar 18, 2024
View reviewed changes
Comment thread internal/ioutil/io.go Outdated
dextrot
dextrot commented Mar 18, 2024
View reviewed changes
Comment thread internal/ioutil/io.go Outdated
@dextrot
chore: update function signature
c504f8f 
Signed-off-by: SKM <131444479+dextrot@users.noreply.github.com>
dextrot
dextrot commented Mar 18, 2024
View reviewed changes
Comment thread internal/ioutil/io.go Outdated
@dextrot
chore: update-comment for UnwrapNopCloser function
30298d1 
Signed-off-by: SKM <131444479+dextrot@users.noreply.github.com>
@dextrot

dextrot commented Mar 18, 2024

Copy link
Copy Markdown
Contributor Author

@dextrot Since we need this PR in the incoming v2.5.0 version, would you like to address the comments?

Done.
Please take a look, if I need to make any further changes.

shizhMSFT
shizhMSFT approved these changes Mar 19, 2024
View reviewed changes

@shizhMSFT shizhMSFT left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. Thanks for contribution!

@shizhMSFT shizhMSFT merged commit bf0d637 into oras-project:main Mar 19, 2024
@shizhMSFT shizhMSFT mentioned this pull request Mar 19, 2024
Merged
Wwwsylvia pushed a commit that referenced this pull request Mar 19, 2024
@shizhMSFT
test: add a new case for UnwrapNopCloser (#725)
8d139f0 
Add a new unit test case by the change to `UnwrapNopCloser` introduced
by #715

---------

Signed-off-by: Shiwei Zhang <shizh@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@shizhMSFT shizhMSFT shizhMSFT approved these changes

@sajayantony sajayantony Awaiting requested review from sajayantony

@SteveLasker SteveLasker Awaiting requested review from SteveLasker

@Wwwsylvia Wwwsylvia Awaiting requested review from Wwwsylvia Wwwsylvia is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Shift Go Support Window to [1.21, 1.22]

3 participants

@dextrot @github-advanced-security @shizhMSFT