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

Upgrade project to use go 1.13 #1135

Merged
merged 1 commit into from
Nov 8, 2019
Merged

Upgrade project to use go 1.13 #1135

merged 1 commit into from
Nov 8, 2019

Conversation

camilamacedo86
Copy link
Member

@camilamacedo86 camilamacedo86 commented Oct 28, 2019 •
edited
Loading

Description
Upgrade the project to work/used 1.13 go and its tests

Motivation
Closes: #1134

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Oct 28, 2019
@camilamacedo86 camilamacedo86 mentioned this pull request Oct 28, 2019
Closed
@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Oct 28, 2019
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Oct 28, 2019
@camilamacedo86 camilamacedo86 changed the title add go 1.13 to travis Upgrade project to use go 1.13 Oct 28, 2019
@k8s-ci-robot k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 28, 2019
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Oct 28, 2019
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 28, 2019
@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 Oct 28, 2019
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 28, 2019
@camilamacedo86 camilamacedo86 mentioned this pull request Nov 5, 2019
Closed
@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 Nov 5, 2019
DirectXMan12
DirectXMan12 requested changes Nov 5, 2019
View reviewed changes
Copy link
Contributor

@DirectXMan12 DirectXMan12 left a comment

Choose a reason for hiding this comment

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

one comment inline, otherwise:

We're going to have to pull the trigger on moving stuff to 1.13 eventually, and it's been a few months now. The only blocker is k/k moving, but I don't think we can wait forever on that, and the issues affecting k/k don't have any bearing on us.

I propose a scream test -- if anyone has any serious issues, we'll hear about it shortly.

We'll need a corresponding update to prow, otherwise we'll bork things.

testdata/project-v2/go.mod Show resolved Hide resolved
@camilamacedo86
Copy link
Member Author

camilamacedo86 commented Nov 7, 2019 •
edited
Loading

H @DirectXMan12,

All done 👍 Also, shows that k8s will be upgraded as well uhu here.

@mengqiy
Copy link
Member

mengqiy commented Nov 8, 2019

k/k is now on golang1.13 with kubernetes/kubernetes#82809 merged.

@camilamacedo86
Copy link
Member Author

Hi @mengqiy and @DirectXMan12,

Let's merge this one? If not, let me know what is missing for we are able to move forward.

@camilamacedo86
Copy link
Member Author

/assign @droot

@camilamacedo86
Copy link
Member Author

/assign @DirectXMan12

@camilamacedo86
Copy link
Member Author

/assign @mengqiy

mengqiy
mengqiy approved these changes Nov 8, 2019
View reviewed changes
Copy link
Member

@mengqiy mengqiy left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 8, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: camilamacedo86, mengqiy

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 Nov 8, 2019
@k8s-ci-robot k8s-ci-robot merged commit 9eeec12 into kubernetes-sigs:master Nov 8, 2019
@DirectXMan12
Copy link
Contributor

k/k is now on golang1.13 with kubernetes/kubernetes#82809 merged.

Full steam ahead on the 1.13 train, then 🚂

@Adirio Adirio mentioned this pull request Nov 11, 2019
Merged
Adirio
Adirio reviewed Nov 11, 2019
View reviewed changes
docs/book/src/cronjob-tutorial/testdata/project/go.mod

require (
github.com/go-logr/logr v0.1.0
github.com/robfig/cron v1.2.0
golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2 // indirect
Copy link
Contributor

Choose a reason for hiding this comment

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

My IDE is inserting this line automatically when I pulled this PR. Does it really need to be removed?

Copy link
Member Author

Choose a reason for hiding this comment

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

It will be removed when we do a go mod tidy than it should not be committed.

testdata/project-v2/go.mod

require (
github.com/go-logr/logr v0.1.0
github.com/onsi/ginkgo v1.6.0
github.com/onsi/gomega v1.4.2
golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2 // indirect
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above.

@Adirio
Copy link
Contributor

Adirio commented Nov 11, 2019

@camilamacedo86 @DirectXMan12 @mengqiy @droot

@camilamacedo86 camilamacedo86 deleted the go-up branch November 11, 2019 13:46
@ncdc ncdc mentioned this pull request Nov 11, 2019
Merged
@camilamacedo86 camilamacedo86 mentioned this pull request Feb 8, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Adirio Adirio Adirio left review comments

@mengqiy mengqiy mengqiy approved these changes

@DirectXMan12 DirectXMan12 Awaiting requested review from DirectXMan12

Assignees

@DirectXMan12 DirectXMan12

@droot droot

@mengqiy mengqiy

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. 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.

Unable to contribute with using go 1.13
6 participants
@camilamacedo86 @mengqiy @k8s-ci-robot @DirectXMan12 @Adirio @droot