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

Bump go version to 1.16 #55

Merged
merged 1 commit into from
Apr 9, 2021
Merged

Bump go version to 1.16 #55

merged 1 commit into from
Apr 9, 2021

Conversation

ArangoGutierrez
Copy link
Contributor

@ArangoGutierrez ArangoGutierrez commented Apr 8, 2021 •
edited
Loading

This patch updates:

  • Go to 1.16.3
  • Run go mod tidy

Signed-off-by: Carlos Eduardo Arango Gutierrez carangog@redhat.com

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Apr 8, 2021
@k8s-ci-robot k8s-ci-robot requested review from marquiz and zvonkok April 8, 2021 20:15
@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 8, 2021
marquiz
marquiz reviewed Apr 8, 2021
View reviewed changes
config/manager/kustomization.yaml Outdated
Comment on lines 15 to 16
newName: quay.io/eduardoarango/node-feature-discovery-operator
newTag: v0.2.0-22-ge57fe01f-dirty
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't change these, please

Copy link
Contributor Author

Choose a reason for hiding this comment

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

damm kustomize... sorry

marquiz
marquiz reviewed Apr 8, 2021
View reviewed changes
config/manager/manager.yaml Outdated
@@ -30,7 +30,7 @@ spec:
capabilities:
drop: ["ALL"]
command:
- /manager
- /node-feature-discovery-operator
Copy link
Contributor

Choose a reason for hiding this comment

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

Unrelated

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is fixed on #53 , had to use it here to.
after merging #53 this line won't be needed

marquiz
marquiz requested changes Apr 8, 2021
View reviewed changes
Copy link
Contributor

@marquiz marquiz left a comment

Choose a reason for hiding this comment

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

Remember to also submit a accompanying PR against https://github.com/kubernetes/test-infra

@ArangoGutierrez
Copy link
Contributor Author

Remember to also submit a accompanying PR against https://github.com/kubernetes/test-infra

working on that

@ArangoGutierrez ArangoGutierrez mentioned this pull request Apr 8, 2021
Merged
@ArangoGutierrez
Copy link
Contributor Author

kubernetes/test-infra#21726

@ArangoGutierrez ArangoGutierrez force-pushed the devel/go116 branch 3 times, most recently from 7a28c5d to e028f46 Compare April 9, 2021 03:33
marquiz
marquiz reviewed Apr 9, 2021
View reviewed changes
Copy link
Contributor

@marquiz marquiz left a comment

Choose a reason for hiding this comment

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

Sorry to nag about the commit message as it's not aligned with the content. Could you update it, e.g. to something like the PR title Bump go version and tidy up go.sum

@ArangoGutierrez
Copy link
Contributor Author

Bump go version and tidy up go.sum

done, now commit has a good and descriptive message

@marquiz
Copy link
Contributor

marquiz commented Apr 9, 2021

commit has a good and descriptive message

Don't see any change there 🤔

@ArangoGutierrez ArangoGutierrez requested a review from marquiz April 9, 2021 12:32
@marquiz
Copy link
Contributor

marquiz commented Apr 9, 2021

Don't see any change there 🤔

Now I see it. I just think that subject of the PR ("Bump go version") would be far better subject to the commit message than the current "update dependencies". Which is quite misleading as this patch does not really udpate any dependencies

@ArangoGutierrez
bump go version to 1.16
fdfaee8 
Bump go version to 1.16 and run go mod tidy

Signed-off-by: Carlos Eduardo Arango Gutierrez <carangog@redhat.com>
@ArangoGutierrez
Copy link
Contributor Author

Don't see any change there thinking

Now I see it. I just think that subject of the PR ("Bump go version") would be far better subject to the commit message than the current "update dependencies". Which is quite misleading as this patch does not really udpate any dependencies

got it

@ArangoGutierrez ArangoGutierrez changed the title Bump go version Bump go version to 1.16 Apr 9, 2021
marquiz
marquiz approved these changes Apr 9, 2021
View reviewed changes
Copy link
Contributor

@marquiz marquiz left a comment

Choose a reason for hiding this comment

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

Thanks. Now we're golang 1.16 everywhere
/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 9, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ArangoGutierrez, marquiz

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:
  • OWNERS [ArangoGutierrez,marquiz]

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 merged commit 002a957 into kubernetes-sigs:master Apr 9, 2021
@marquiz marquiz mentioned this pull request Oct 26, 2021
Closed
13 tasks
@ArangoGutierrez ArangoGutierrez deleted the devel/go116 branch March 13, 2023 10:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marquiz marquiz marquiz approved these changes

@zvonkok zvonkok Awaiting requested review from zvonkok

Assignees

@marquiz marquiz

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.
3 participants
@ArangoGutierrez @marquiz @k8s-ci-robot