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.

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

Switch to golang 1.11.5 #6798

Merged
merged 3 commits into from
Apr 22, 2019
Merged

Switch to golang 1.11.5 #6798

merged 3 commits into from
Apr 22, 2019

Conversation

justinsb
Copy link
Member

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Apr 20, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: justinsb

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 20, 2019
@rdrgmnzs rdrgmnzs mentioned this pull request Apr 20, 2019
Merged
@rdrgmnzs
Copy link
Contributor

@justinsb we’ll need to remove the 1.10 tests and run gofmt against the existing code base to deal with the formatting changes that were introduced in 1.11

@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Apr 20, 2019
@justinsb
Source gofmt from bazel go
bf4c4a1 
Using the approach from k8s.io/repo-infra.  This avoids problems where
we are testing with a different version of go than we are building
with.
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Apr 21, 2019
justinsb added a commit to justinsb/test-infra that referenced this pull request Apr 21, 2019
@justinsb
kops: move gofmt job to use bazel image
3d728a2 
So that we can source the correct version of go & gofmt, it makes
sense to use the gofmt from the bazel @go_sdk, as done in
k8s.io/repo-infra.

This requires that we use the bazel image.

Related PR kubernetes/kops#6798
@justinsb justinsb mentioned this pull request Apr 21, 2019
Merged
@rdrgmnzs
Copy link
Contributor

/test pull-kops-verify-gofmt

justinsb added a commit to justinsb/test-infra that referenced this pull request Apr 22, 2019
@justinsb
kops: move gofmt job to use bazel image
4fcacab 
So that we can source the correct version of go & gofmt, it makes
sense to use the gofmt from the bazel go_sdk, as done in
k8s.io/repo-infra.

This requires that we use the bazel image.

Related PR kubernetes/kops#6798
@justinsb
Copy link
Member Author

justinsb commented Apr 22, 2019 •
edited
Loading

Working on fixing the gofmt test job, it's currently locked on a particular go version: kubernetes/test-infra#12298

@justinsb
Copy link
Member Author

pull-kops-verify-gofmt job change was merged upstream, so...

/retest

@justinsb
travis: Create new travis-ci makefile target, run that
69e0b26 
This lets us exclude the gofmt task which now requires bazel and also
now runs in a separate test.

Continuing with this should allow us to have a faster travis-ci, that
should also give us better coverage (we really want travis to test osx
& windows builds)
@rdrgmnzs
Copy link
Contributor

Just food for thought, would it be more maintainable to install Bazel in the Travis image using before_install like in https://github.com/bazelbuild/rules_go/blob/master/.travis.yml instead of having a separate CI test for it instead?

@justinsb
Copy link
Member Author

justinsb commented Apr 22, 2019 •
edited
Loading

@rdrgmnzs I did think about it, but it's pretty slow to download bazel every time, and I think generally I'd rather move testing out of travis, and into prow. I imagine we end up with travis primarily testing that kops itself builds on osx & linux (& windows?), on multiple go versions. That's much harder to do in prow.

@rdrgmnzs
Copy link
Contributor

Makes sense, thanks @justinsb !

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 22, 2019
@k8s-ci-robot k8s-ci-robot merged commit 732121d into kubernetes:master Apr 22, 2019
@justinsb justinsb mentioned this pull request Apr 22, 2019
Merged
k8s-ci-robot added a commit that referenced this pull request Apr 22, 2019
@k8s-ci-robot
Merge pull request #6808 from justinsb/cherrypick_6798_release-1.13
36b12a3 
Cherry pick of #6798 onto release-1.13
@justinsb justinsb mentioned this pull request Apr 23, 2019
Merged
k8s-ci-robot added a commit that referenced this pull request Apr 23, 2019
@k8s-ci-robot
Merge pull request #6815 from justinsb/cherry_pick_6799
50506f0 
Cherry pick of #6798 and #6799 onto release-1.14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rdrgmnzs rdrgmnzs Awaiting requested review from rdrgmnzs

@robinpercy robinpercy Awaiting requested review from robinpercy

Assignees

@rdrgmnzs rdrgmnzs

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cherry-pick cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@justinsb @k8s-ci-robot @rdrgmnzs