Skip to content

update CI to test with supported Go versions (1.15, 1.16) #1839

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

Closed
wants to merge 3 commits into from

Conversation

aldas
Copy link
Contributor

@aldas aldas commented Apr 7, 2021

update CI to test with supported Go versions (1.15, 1.16)

@aldas aldas force-pushed the test_with_supported_versions branch from fcb02b6 to d8f72e1 Compare April 7, 2021 20:23
@aldas aldas requested a review from lammel April 9, 2021 04:35
Copy link
Contributor

@lammel lammel left a comment

Choose a reason for hiding this comment

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

If we remove Go 1.12 and 1.13 from the tests and README, we should also cleanup all code using custom code and build tags that were added to support those releases.

I thnk we should deprecated Go 1.12 with our next minor release echo v4.3 and
to only Go 1.15+Go 1.16 with v5. Focus here is to minimize developer disruption.

We have quite some Go version specific code currently that we should cleanup too.

For Go 1.12:

echo# rg '1.12([^0-9-]|$)'
middleware/csrf_samesite_test.go
14:// Test for SameSiteModeNone moved to separate file for Go 1.12 support

middleware/csrf_samesite_1.12.go
10:	// SameSiteNoneMode required to be redefined for Go 1.12 support (see #1524)

Makefile
32:goversion ?= "1.12"
33:test_version: ## Run tests inside Docker with given version (defaults to 1.12 oldest supported). Example: make test_version goversion=1.13

middleware/csrf_samesite.go
10:	// SameSiteNoneMode required to be redefined for Go 1.12 support (see #1524)

For Go 1.13:

echo# rg '1.13([^0-9-]|$)'
CHANGELOG.md
32:* Middleware: New timeout middleware implementation for go1.13+ (#1743, )

middleware/csrf_samesite_test.go
1:// +build go1.13

middleware/timeout.go
1:// +build go1.13

middleware/timeout_test.go
1:// +build go1.13

middleware/csrf_samesite.go
1:// +build go1.13

middleware/csrf_samesite_1.12.go
1:// +build !go1.13

echo_go1.13_test.go
1:// +build go1.13

Makefile
33:test_version: ## Run tests inside Docker with given version (defaults to 1.12 oldest supported). Example: make test_version goversion=1.13

echo.go
888:// Unwrap satisfies the Go 1.13 error wrapper interface.

Nothing for Go 1.14

echo# rg '1.14([^0-9-]|$)'
README.md
20:- 1.14+

So probably this PR should only add Go 1.16 and we should do 2 more PRs to remove Go 1.12 and Go 1.13. What are your thoughts on that?

@aldas
Copy link
Contributor Author

aldas commented Apr 13, 2021

I'll add below 1.15 cleanup here as extra commits.

* go1.14 (released 2020/02/25) - new binder time.duration errors message tests
* go1.13 (released 2019/09/03) - timeout middleware, HTTPError unwrapping related
* go1.12 (released 2019/02/25) - csrf middleware related
* go1.11 (released 2018/08/24) - proxy middleware related
@codecov
Copy link

codecov bot commented Apr 13, 2021

Codecov Report

Merging #1839 (38bd514) into master (8da8e16) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1839   +/-   ##
=======================================
  Coverage   89.57%   89.57%           
=======================================
  Files          32       31    -1     
  Lines        2686     2686           
=======================================
  Hits         2406     2406           
  Misses        180      180           
  Partials      100      100           
Impacted Files Coverage Δ
middleware/csrf.go 80.28% <ø> (ø)
middleware/timeout.go 100.00% <ø> (ø)
middleware/proxy.go 66.66% <100.00%> (+6.20%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8da8e16...38bd514. Read the comment docs.

@aldas
Copy link
Contributor Author

aldas commented Apr 13, 2021

Remove specific cases/tests to support Go version below 1.15

Version specific code that was removed:

  • go1.14 (released 2020/02/25) - new binder time.duration errors message tests
  • go1.13 (released 2019/09/03) - timeout middleware, HTTPError unwrapping related
  • go1.12 (released 2019/02/25) - csrf middleware related
  • go1.11 (released 2018/08/24) - proxy middleware related

@lammel
Copy link
Contributor

lammel commented Apr 14, 2021

This does look very clean now!
Great work. Docs need to be adjusted too and can also be simplified.
Some older Linux dists are still using Go 1.11 to 1.14 though.

A short snapshot for Debian Linux reveals that Go 1.11 is still in use there in the current stable release:

jessie (oldoldstable) (devel): Go programming language compiler - metapackage
2:1.3.3-1+deb8u2 [security]: all
stretch (oldstable) (devel): Go programming language compiler - metapackage
2:1.7~5: amd64 arm64 armel armhf i386 ppc64el
stretch-backports (devel): Go programming language compiler - metapackage
2:1.11~1~bpo9+1: amd64 arm64 armel armhf i386 mips mipsel ppc64el s390x
buster (stable) (devel): Go programming language compiler - metapackage
2:1.11~1: amd64 arm64 armel armhf i386 mips mipsel ppc64el s390x
buster-backports (devel): Go programming language compiler - metapackage
2:1.14~1~bpo10+1: amd64 arm64 armel armhf i386 mips mipsel ppc64el s390x
bullseye (testing) (golang): Go programming language compiler - metapackage
2:1.15~1: amd64 arm64 armel armhf i386 mips64el mipsel ppc64el s390x
sid (unstable) (golang): Go programming language compiler - metapackage
2:1.15~1: amd64 arm64 armel armhf i386 mips64el mipsel ppc64el riscv64 s390x

So I'm in favor of tagging it for v5 and merging it on the upcoming v5 branch.
Love to here other thoughts too.

@aldas
Copy link
Contributor Author

aldas commented Apr 15, 2021

I do not think using debian as cadence makes sense. Those versions, 1.14 and below, are not getting even security updates anymore by Go release policy.

Each major Go release is supported until there are two newer major releases. For example, Go 1.5 was supported until the Go 1.7 release, and Go 1.6 was supported until the Go 1.8 release. We fix critical problems, including critical security problems, in supported releases as needed by issuing minor revisions (for example, Go 1.6.1, Go 1.6.2, and so on).

https://golang.org/doc/devel/release.html#policy

As far as this change goes (tests excluded) these are places and versions affected:

  • proxy middleware has different implementation for
    • 1.10 and below
    • 1.11 and up
  • CSRF middleware has custom SameSiteNoneMode constant for
    • 1.12 and below
    • 1.13 and up
  • Timeout middleware supports only 1.13 and up

so there is nothing really special from 1.13 and up in Echo.

maybe as compromise it would be ok to have policy that last 3 or 4 releases are supported. This would mean, if Go team release cycle is about 6 months, that we support 6 months or 12 months longer some versions.

@aldas aldas closed this Apr 15, 2021
@aldas aldas deleted the test_with_supported_versions branch May 23, 2021 06:06
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