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 fails for Go 1.15 #1864

Closed
marckhouzam opened this issue Nov 22, 2022 · 7 comments
Closed

CI fails for Go 1.15 #1864

marckhouzam opened this issue Nov 22, 2022 · 7 comments
Labels
admin For general admin tasks to be done usualy by maintainers dependencies Pull requests that update a dependency file

Comments

@marckhouzam
Copy link
Collaborator

As was seen in #1863, the go 1.15 CI is failing with

../../../go/pkg/mod/github.com/kyoh86/richgo@v0.3.11/config/load.go:64:9: undefined: os.ReadFile

Looks like richgo 0.3.11, which was recently released (Nov 15, 2022), requires go 1.16.

@marckhouzam
Copy link
Collaborator Author

I see two options here:

  1. Fix richgo to v0.3.10 until we drop support for Go 1.15
  2. Drop support for Go 1.15 now (as suggested by @umarcor in Update stale.yml #1863 (comment))

I'm ok with either option.

We also need to update our deprecation document as we have agreed we will not do a major Cobra release when removing support for Go versions.

I suggest our support policy for Go should be something like: "We guarantee to support the latest 2 versions of Go, just like the Go project does. To give more flexibility to Cobra consumers, we also won't remove support for older Go versions until a valid reason arises; valid reasons are at the discretion of the maintainers and include but are not limited to, hindering the evolution of the project, limiting the use of CI tools, causing a noticeable added burden of maintenance."

What do you think @jpmcb @umarcor ?

@jpmcb
Copy link
Collaborator

jpmcb commented Nov 22, 2022

Thanks for opening this discussion Marc - I have a few thoughts:

I think we should remove usage of richgo entirely since it only provides us with colored / paged / enriched test output which is non-critical to the cobra library. It also creates a weird barrier to entry to contribute to cobra which I would personally like removed.

If that is unacceptable, I think we should visit deprecating Go 1.15 from cobra. It's been EOL for awhile now so I'm not concerned with breaking consumers of cobra but it does deserve some deep consideration since cobra's posture towards old versions of Go has been "it should just work".

@marckhouzam
Copy link
Collaborator Author

I think we should remove usage of richgo entirely since it only provides us with colored / paged / enriched test output which is non-critical to the cobra library. It also creates a weird barrier to entry to contribute to cobra which I would personally like removed.

You had me at “barrier to entry “ 😜

Also, I personally prefer keeping go versions as long as possible, even if they are EOL. We don't know the reality of the many projects using Cobra so let's help as much as we can.

So +1 to remove our use of richgo. Thanks for thinking outside the box @jpmcb

@marckhouzam
Copy link
Collaborator Author

So @umarcor had the interesting idea of removing richgo for make test and therefore removing the barrier to entry, but to add a new make richtest target to allow those that like the colored output to continue using it. I merged the change.

To make the CI logs easy to read, CI was made to point to make richtest. This means CI still fails with Go 1.15 because of richgo. But @umarcor also posted #1866 to remove Go 1.15 from CI. Note that #1866 just removes the build for Go 1.15, but does not bump the version in go.mod, so we still would allow project to use Go 1.15.

I like the approach but would like @jpmcb's opinion.

And thanks @umarcor for your continued efforts.

@umarcor
Copy link
Contributor

umarcor commented Nov 25, 2022

We also need to update our deprecation document as we have agreed we will not do a major Cobra release when removing support for Go versions.

I suggest our support policy for Go should be something like: "We guarantee to support the latest 2 versions of Go, just like the Go project does. To give more flexibility to Cobra consumers, we also won't remove support for older Go versions until a valid reason arises; valid reasons are at the discretion of the maintainers and include but are not limited to, hindering the evolution of the project, limiting the use of CI tools, causing a noticeable added burden of maintenance."

See #1807.
Related #1658.

@johnSchnake johnSchnake added admin For general admin tasks to be done usualy by maintainers dependencies Pull requests that update a dependency file labels Nov 30, 2022
@thaJeztah
Copy link
Contributor

Whoops; missed this discussion and I already had a branch created / pushed to update the minimum version to go1.17 (#1871). Generally, recommendation is indeed to support last 2 versions, although some project may be "slightly" behind, so where possible 3 versions is "nice" to give projects some time-windows to update (but consider it a "best effort").

I noticed the module that's failing specifies go1.17 as minimum version, which matches "3 versions";

Error: ../../../go/pkg/mod/github.com/kyoh86/richgo@v0.3.11/config/load.go:64:9: undefined: os.ReadFile
note: module requires Go 1.17
Error: Process completed with exit code 2.

@marckhouzam
Copy link
Collaborator Author

marckhouzam commented Jan 3, 2023

I went ahead and merged #1866 which stops running CI on go 1.15 (since CI uses richgo which no longer supports go 1.15) but allows projects to continue using Go 1.15.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
admin For general admin tasks to be done usualy by maintainers dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants