Skip to content

Update golang to 1.19.2 #8622

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 9 commits into from
Oct 31, 2022
Merged

Update golang to 1.19.2 #8622

merged 9 commits into from
Oct 31, 2022
+478 −505

Conversation

jean-edouard
Copy link
Contributor

@jean-edouard jean-edouard commented Oct 18, 2022 •
edited
Loading

What this PR does / why we need it:
This PR update the go compiler from version 1.17 to the latest 1.19 release.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Special notes for your reviewer:
Using io/ioutil now breaks the linter, so I replaced all uses of it with functions from io/os

Release note:

Built with golang 1.19

@kubevirt-bot kubevirt-bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note Denotes a PR that will be considered when it comes time to generate release notes. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. size/M labels Oct 18, 2022
@jean-edouard jean-edouard changed the title WIP: Update golang to 1.19.2 Update golang to 1.19.2 Oct 18, 2022
@kubevirt-bot kubevirt-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 18, 2022
@jean-edouard
Copy link
Contributor Author

/retest

rmohr
rmohr reviewed Oct 19, 2022
View reviewed changes
go.mod
@@ -207,4 +207,4 @@ replace (
sigs.k8s.io/controller-runtime => sigs.k8s.io/controller-runtime v0.6.2
)

go 1.17
go 1.19
Copy link
Member

Choose a reason for hiding this comment

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

Bazel will still use 1.17. You have to update the WORKSPACE dependencies (rules_go and gazelle) and bump the golang version: https://github.com/bazelbuild/bazel-gazelle#running-gazelle-with-bazel.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, thank you, I knew it couldn't be that easy, working on it now!

@rmohr
Copy link
Member

rmohr commented Oct 19, 2022

Using io/ioutil now breaks the linter, so I replaced all uses of it with functions from io/os

Could you share the issue it produces? Maybe that is because bazel was not updated to 1.19?

@rmohr
Copy link
Member

rmohr commented Oct 19, 2022

/hold

bazel is not bumpte to 1.19

@kubevirt-bot kubevirt-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 19, 2022
@jean-edouard
Copy link
Contributor Author

Using io/ioutil now breaks the linter, so I replaced all uses of it with functions from io/os

Could you share the issue it produces? Maybe that is because bazel was not updated to 1.19?

Here: https://prow.ci.kubevirt.io/view/gs/kubevirt-prow/pr-logs/pull/kubevirt_kubevirt/8622/pull-kubevirt-code-lint/1582365920414666752

@iholder101
Copy link
Contributor

/cc
p.s. yay, generics! 😄

@kubevirt-bot kubevirt-bot added kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/build-change Categorizes PRs as related to changing build files of virt-* components labels Oct 24, 2022
@rmohr
Copy link
Member

rmohr commented Oct 24, 2022

@jean-edouard pushed the necessary bazel changes to your branch. I think we need to bump the builder image once more.

@jean-edouard
Copy link
Contributor Author

@jean-edouard pushed the necessary bazel changes to your branch. I think we need to bump the builder image once more.

Wow, thank you so much! I'll take care of the bump asap today

rmohr
rmohr reviewed Oct 24, 2022
View reviewed changes
WORKSPACE Outdated
@@ -4,6 +4,8 @@ load("//third_party:deps.bzl", "deps")

deps()

load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
Copy link
Member

Choose a reason for hiding this comment

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

ok, that one is not needed. An oversight on my part. It is imported below already.

@rmohr
Copy link
Member

rmohr commented Oct 28, 2022

/lgtm
/approve

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Oct 28, 2022
@kubevirt-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rmohr

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kubevirt-bot kubevirt-bot added approved Indicates a PR has been approved by an approver from all required OWNERS files. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Oct 28, 2022
@kubevirt-bot kubevirt-bot removed lgtm Indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Oct 28, 2022
@rmohr
Copy link
Member

rmohr commented Oct 28, 2022

/lgtm

@kubevirt-bot kubevirt-bot added lgtm Indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Oct 28, 2022
jean-edouard and others added 9 commits October 31, 2022 09:43
@jean-edouard
Update golang to 1.19.2
fe1eed6 
Signed-off-by: Jed Lejosne <jed@redhat.com>
@jean-edouard
Remove all uses of io/ioutil
f1917d3 
Signed-off-by: Jed Lejosne <jed@redhat.com>
@rmohr @jean-edouard
Bump golang toolchain
80a216c 
Signed-off-by: Roman Mohr <rmohr@google.com>
@rmohr @jean-edouard
Bump bazel to 5.3.1
d8e08f6 
Signed-off-by: Roman Mohr <rmohr@google.com>
@rmohr @jean-edouard
Reformat code
9fe5b02 
Signed-off-by: Roman Mohr <rmohr@google.com>
@rmohr @jean-edouard
Switch to external package rules for virt-launcher
d627a12 
Signed-off-by: Roman Mohr <rmohr@google.com>
@rmohr @jean-edouard
Bump bazel version in builder image
9e15eca 
Signed-off-by: Roman Mohr <rmohr@google.com>
@jean-edouard
Update builder image and misc fixes
1633182 
Signed-off-by: Jed Lejosne <jed@redhat.com>
@rmohr @jean-edouard
Set expected coverage profile format
3e87375 
rules_go finally supports lcov integration, but we still need the legacy
coverage format for goveralls integration. Requesting it explicitly.

Signed-off-by: Roman Mohr <rmohr@google.com>
@kubevirt-bot kubevirt-bot removed lgtm Indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Oct 31, 2022
@rmohr
Copy link
Member

rmohr commented Oct 31, 2022

/lgtm

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Oct 31, 2022
@jean-edouard
Copy link
Contributor Author

/retest

@kubevirt-bot
Copy link
Contributor

@jean-edouard: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-kubevirt-e2e-k8s-1.22-sig-performance 3e87375 link false /test pull-kubevirt-e2e-k8s-1.22-sig-performance

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@kubevirt-bot kubevirt-bot merged commit 03e0571 into kubevirt:main Oct 31, 2022
@Longchuanzheng Longchuanzheng mentioned this pull request Mar 5, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rmohr rmohr

@mhenriks mhenriks

@rthallisey rthallisey

@iholder101 iholder101

Assignees

@rmohr rmohr

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/monitoring dco-signoff: yes Indicates the PR's author has DCO signed all their commits. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/build-change Categorizes PRs as related to changing build files of virt-* components lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/network size/XL
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@jean-edouard @rmohr @iholder101 @kubevirt-bot