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

ci(*): Upgrade minimum version to Go 1.16 #916

Merged
merged 2 commits into from
Oct 20, 2021
Merged

ci(*): Upgrade minimum version to Go 1.16 #916

merged 2 commits into from
Oct 20, 2021

Conversation

Juneezee
Copy link
Member

@Juneezee Juneezee commented Oct 20, 2021

  1. Drop Go 1.15 and add Go 1.17 to GitHub Actions workflows.
    2. Remove tidy as the dependency of build target and add another make tidy-all step to build-test.yml. This is required otherwise make build-all will report an error when the current go.mod file indicates a higher Go version than the current installed version.
  2. The io/ioutil package has been deprecated in Go 1.16 (See https://golang.org/doc/go1.16#ioutil). This PR replaces the existing io/ioutil functions with their new definitions in io and os packages.

EDIT: Revert Makefile changes

@Xuanwo
Copy link
Contributor

Xuanwo commented Oct 20, 2021

Thanks for your contributions as first.

We have set a GSP about our release policy: https://github.com/beyondstorage/go-storage/blob/master/docs/rfcs/15-release-policy.md

So upgrade to 1.17 is not suitable for us.

Upgrade to 1.16 is OK though.

We have a discussion about it at our forum:https://forum.beyondstorage.io/t/topic/244

Do you want to implement it?

GitHub
A vendor-neutral storage library for Golang: Write once, run on every storage service. - go-storage/15-release-policy.md at master · beyondstorage/go-storage
BeyondStorage
GSP-15: Release policy has declared our target golang version policy: storage SHOULD be compatible with the last two golang major versions Assuming the current version is go 1.14 Developer SHOULD develop with go 1.13 OR go 1.14 CI should be passed on BOTH go 1.13 AND go 1.14 Any error/bug report on go 1.12 MAY mark as wontfix New features included in go 1.14 SHOULD NOT be included As go 1.17 has been released at 2021-08-16, maybe it’s time for us to upgrade our minimum golang version fro...

@Xuanwo
Copy link
Contributor

Xuanwo commented Oct 20, 2021

BTW, it's OK to me to just add go 1.17 support in this PR. And we can discuss the upgrade to 1.16 in another one.

@Juneezee
Copy link
Member Author

Juneezee commented Oct 20, 2021

@Xuanwo Hi! Sure, I can upgrade it to Go 1.16. But is it okay to drop 1.15 and start to support 1.17 in GitHub Actions? Since Go 1.15 has reached end-of-life
image
Source: https://endoflife.date/go

endoflife.date
Check End of Life, Support Schedule, and release timelines for Go. endoflife.date/golang. Go release policy. Go release schedule. Go end of life.

@Xuanwo
Copy link
Contributor

Xuanwo commented Oct 20, 2021

Yes, we can drop go 1.15 in github action. 😝

@Xuanwo
Copy link
Contributor

Xuanwo commented Oct 20, 2021

Maybe we don't need to change the Makefile if we use go 1.16 in go.mod?

This commit also drops Go 1.15 and adds Go 1.17 to GitHub Actions
workflows.

Signed-off-by: Eng Zer Jun <[email protected]>
The io/ioutil package has been deprecated as of Go 1.16, see
https://golang.org/doc/go1.16#ioutil. This commit replaces the existing
io/ioutil functions with their new definitions in io and os packages.

Signed-off-by: Eng Zer Jun <[email protected]>
@Juneezee
Copy link
Member Author

Maybe we don't need to change the Makefile if we use go 1.16 in go.mod?

🙈 Oops...I missed that one. I have reverted the Makefile changes. Just a side note, if we plan to use a higher Go version in go.mod and still support an older Go version in GitHub Actions in the future, then go mod tidy will return an error.

@Juneezee
Copy link
Member Author

Juneezee commented Oct 20, 2021

@Xuanwo The failing test Services Test Ftp / Integration Test (asf/mina-ftpserver) has been failing repeated and does not seem to be a flaky test. The first time it failed is at this commit 006447b (#906)

Failures:

  * /home/runner/work/go-storage/go-storage/services/ftp/tests/storage_test.go 
  Line 14:
  Expected: nil
  Actual:   'read: Storager ftp {URL: 127.0.0.1:2121, User: admin, WorkDir: /}, [9825720d-225b-401d-916e-e4ae88320dd8]: service internal, 426 Data connection error.'

@Xuanwo
Copy link
Contributor

Xuanwo commented Oct 20, 2021

OK, let's merge this PR and than fix the test.

@Xuanwo Xuanwo changed the title Upgrade to Go 1.17 ci(*): Upgrade minimum version to Go 1.16 Oct 20, 2021
@Xuanwo Xuanwo merged commit b82c62e into beyondstorage:master Oct 20, 2021
@Xuanwo
Copy link
Contributor

Xuanwo commented Oct 20, 2021

PR merged, thanks!

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.

2 participants