Set go version to 1.17 to match CI #10581
Conversation
ddaa738 to
847eb7c
Compare
|
CI Has this error: The PR doesn't contain any modification to go files, I guess this is normal (but probably should not fail ?) |
|
likely fixed once #10575 is merged - golint is deprecated. |
|
For reference, the version number listed in go.mod dictates the minimum version of Go you can use to compile the project. It does not have to match the same version in CI - in fact it can be several versions behind. However in the case described in #10580, it sounds like Helm cannot be compiled against Go 1.16 due to an issue with a transitive dependency, so bumping go.mod to a version that does compile seems fine to me. |
There was a problem hiding this comment.
LGTM once we merge #10575 and this PR is rebased to ensure everything still works
|
I think, the CI should use the minimum version of Go you can use to compile the project, so this situation doesn't happen. |
I agree that we should run the test suite against all supported versions of Go (and we'd appreciate a PR to do so next time!). But I don't think we should release new versions of Helm compiled against the oldest supported version. Newer versions of Go contain performance improvements and bug fixes, many of which will benefit Helm users. For example, Go 1.17 brought improvements to the compiler, resulting in an average performance improvement of 5% and a reduction in binary size of ~2% (source). That is why we compile Helm against the latest stable version of Go. |
|
Can you please rebase? Thanks |
Signed-off-by: Samuel Maftoul <samuel.maftoul@gmail.com>
847eb7c to
850da38
Compare
|
@bacongobbler Done ! |
Closes #10580
What this PR does / why we need it:
Bump go version to 1.17 to match CI. It's needed because building with 1.16 is broken and because the CI uses 1.17.