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

remove go 1.17 from CI tests job #1263

Closed

Conversation

alexandear
Copy link
Contributor

Fixes Or Enhances

Removes Go 1.17 from the test CI job to speed up pipeline execution.

Testing with Go 1.17 is not needed because the minimum supported version is Go 1.18, as stated in the go.mod.

@go-playground/validator-maintainers

@alexandear alexandear requested a review from a team as a code owner May 16, 2024 15:13
@alexandear alexandear changed the title remove go 1.17 from ci build tests remove go 1.17 from CI tests job May 16, 2024
@coveralls
Copy link

coveralls commented May 16, 2024

Coverage Status

coverage: 74.243%. remained the same
when pulling c1314b8 on alexandear:ci-remove-go-1-17
into e20b948 on go-playground:master.

@deankarn
Copy link
Contributor

deankarn commented Jun 1, 2024

@alexandear this is a common misconception, but thank you for the PR.

It only became mandatory in Go 1.21+ Before Go 1.21, the directive was advisory only; now it is a mandatory requirement asserted in the Go Mod Reference https://go.dev/doc/modules/gomod-ref

And so versions prior to Go 1.21, including Go 1.17 are still supported by this package and the CI test is here to ensure no breaking changes are made to the package and Go 1.17 users.

It's an unfortunate and confusing state of affairs :(

Also see this previous comment with similar/related content #1225 (comment)

@alexandear
Copy link
Contributor Author

@deankarn thanks for detailed response. Maybe it's better to set go 1.17 in go.mod? So, everyone knows Go 1.17 is supported by the library.

@alexandear alexandear closed this Jun 1, 2024
@alexandear alexandear deleted the ci-remove-go-1-17 branch June 1, 2024 15:41
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.

None yet

3 participants