Skip to content

Fix replace go mod of client-go#3668

Merged
tekton-robot merged 1 commit intotektoncd:masterfrom
howardjohn:fix-replace-go-mod
Jan 27, 2021
Merged

Fix replace go mod of client-go#3668
tekton-robot merged 1 commit intotektoncd:masterfrom
howardjohn:fix-replace-go-mod

Conversation

@howardjohn
Copy link
Copy Markdown
Contributor

@howardjohn howardjohn commented Jan 8, 2021

Changes

/kind misc

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

  • [N/A] Includes tests (if functionality changed/added)
  • [N/A] Includes docs (if user facing)
  • Commit messages follow commit message best practices
  • Release notes block has been filled in or deleted (only if no user facing changes)

See the contribution guide for more details.

Double check this list of stuff that's easy to miss:

Reviewer Notes

If API changes are included, additive changes must be approved by at least two OWNERS and backwards incompatible changes must be approved by more than 50% of the OWNERS, and they must first be added in a backwards compatible way.

Release Notes

@tekton-robot tekton-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/misc Categorizes issue or PR as a miscellaneuous one. labels Jan 8, 2021
@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla bot commented Jan 8, 2021 •
edited
Loading

CLA Signed

The committers are authorized under a signed CLA.

@tekton-robot tekton-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jan 8, 2021
@tekton-robot
Copy link
Copy Markdown
Collaborator

tekton-robot commented Jan 8, 2021

Hi @howardjohn. Thanks for your PR.

I'm waiting for a tektoncd member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

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.

howardjohn added a commit to howardjohn/kubernetes-test-infra that referenced this pull request Jan 8, 2021
@howardjohn
Drop boskos import and client-go@v11
c1b91a9 
This contains a `replace` for tektoncd, will not be needed when
tektoncd/pipeline#3668 is merged. I am opening
the PR now for early feedback.

This is an attempt at fixing
kubernetes#20421. This is done by
making kubetest a submodule. Alternatives are to remove it entirely (if
its fully replaced with kubetest2) or move it to its own repo.
@howardjohn howardjohn mentioned this pull request Jan 8, 2021
Closed
@vdemeester
Copy link
Copy Markdown
Member

vdemeester commented Jan 12, 2021

@howardjohn any reason for this change ? Overall I feel it's better to not have replace so this works for me if it all works out 😉

/cc @tektoncd/core-maintainers @mattmoor

/ok-to-test
/hold
(Holding for making sure we do that after 0.20.0)

@tekton-robot tekton-robot requested review from a team and mattmoor January 12, 2021 16:46
@tekton-robot tekton-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jan 12, 2021
@howardjohn
Copy link
Copy Markdown
Contributor Author

howardjohn commented Jan 12, 2021

The motivation is I want to drop replace on our project and tekton is about 4 imports down the chain of projects that need to be updated to get mine to be able to drop it 🙂

@vdemeester
Copy link
Copy Markdown
Member

vdemeester commented Jan 12, 2021 

diff --git a/vendor/modules.txt b/vendor/modules.txt
index 3eacffc6b..dec0d4705 100644
--- a/vendor/modules.txt
+++ b/vendor/modules.txt
@@ -568,7 +568,7 @@ gopkg.in/evanphx/json-patch.v4
 gopkg.in/inf.v0
 # gopkg.in/yaml.v2 v2.3.0
 gopkg.in/yaml.v2
-# k8s.io/api v0.18.12 => k8s.io/api v0.18.12
+# k8s.io/api v0.18.12
 k8s.io/api/admission/v1
 k8s.io/api/admissionregistration/v1
 k8s.io/api/admissionregistration/v1beta1
@@ -610,7 +610,7 @@ k8s.io/api/settings/v1alpha1
 k8s.io/api/storage/v1
 k8s.io/api/storage/v1alpha1
 k8s.io/api/storage/v1beta1
-# k8s.io/apiextensions-apiserver v0.18.12 => k8s.io/apiextensions-apiserver v0.18.12
+# k8s.io/apiextensions-apiserver v0.18.12
 k8s.io/apiextensions-apiserver/pkg/apis/apiextensions
 k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1
 k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1
@@ -625,7 +625,7 @@ k8s.io/apiextensions-apiserver/pkg/client/informers/externalversions/apiextensio
 k8s.io/apiextensions-apiserver/pkg/client/informers/externalversions/internalinterfaces
 k8s.io/apiextensions-apiserver/pkg/client/listers/apiextensions/v1
 k8s.io/apiextensions-apiserver/pkg/client/listers/apiextensions/v1beta1
-# k8s.io/apimachinery v0.18.12 => k8s.io/apimachinery v0.18.12
+# k8s.io/apimachinery v0.18.12
 k8s.io/apimachinery/pkg/api/equality
 k8s.io/apimachinery/pkg/api/errors
 k8s.io/apimachinery/pkg/api/meta
@@ -673,7 +673,7 @@ k8s.io/apimachinery/pkg/version
 k8s.io/apimachinery/pkg/watch
 k8s.io/apimachinery/third_party/forked/golang/json
 k8s.io/apimachinery/third_party/forked/golang/reflect
-# k8s.io/client-go v11.0.1-0.20190805182717-6502b5e7b1b5+incompatible => k8s.io/client-go v0.18.12
+# k8s.io/client-go v0.18.12
 k8s.io/client-go/discovery
 k8s.io/client-go/discovery/fake
 k8s.io/client-go/dynamic
@@ -886,7 +886,7 @@ k8s.io/client-go/util/jsonpath
 k8s.io/client-go/util/keyutil
 k8s.io/client-go/util/retry
 k8s.io/client-go/util/workqueue
-# k8s.io/code-generator v0.18.12 => k8s.io/code-generator v0.18.12
+# k8s.io/code-generator v0.18.12
 k8s.io/code-generator/cmd/client-gen
 k8s.io/code-generator/cmd/client-gen/args
 k8s.io/code-generator/cmd/client-gen/generators
@@ -922,7 +922,7 @@ k8s.io/gengo/types
 k8s.io/klog
 # k8s.io/klog/v2 v2.0.0
 k8s.io/klog/v2
-# k8s.io/kube-openapi v0.0.0-20200410145947-bcb3869e6f29 => k8s.io/kube-openapi v0.0.0-20200410145947-bcb3869e6f29
+# k8s.io/kube-openapi v0.0.0-20200410145947-bcb3869e6f29
 k8s.io/kube-openapi/cmd/openapi-gen
 k8s.io/kube-openapi/cmd/openapi-gen/args
 k8s.io/kube-openapi/pkg/common

(ran go mod vendor and go mod tidy to make the modules.txt up-to-date)

@howardjohn
Copy link
Copy Markdown
Contributor Author

howardjohn commented Jan 19, 2021

Thanks @vdemeester , updated with the command you suggested

vdemeester
vdemeester reviewed Jan 20, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

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

/lgtm
/cc @tektoncd/core-maintainers

@tekton-robot tekton-robot requested a review from a team January 20, 2021 09:14
@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 20, 2021
@tekton-robot
Copy link
Copy Markdown
Collaborator

tekton-robot commented Jan 20, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ImJasonH

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

The pull request process is described here

Details 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

@tekton-robot tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 20, 2021
@howardjohn
Copy link
Copy Markdown
Contributor Author

howardjohn commented Jan 20, 2021

I am not actually sure the process to get things merged, but seems there is a hold. Let me know if I need to do anything to remove it. Thanks!

@vdemeester
Copy link
Copy Markdown
Member

vdemeester commented Jan 20, 2021

I am not actually sure the process to get things merged, but seems there is a hold. Let me know if I need to do anything to remove it. Thanks!

I am just waiting for at least another owner to approve/lgtm to remove the hold 😉
cc @bobcatfish @sbwsg @afrittoli

@howardjohn
Copy link
Copy Markdown
Contributor Author

howardjohn commented Jan 27, 2021

Any update @bobcatfish @sbwsg @afrittoli?

afrittoli
afrittoli reviewed Jan 27, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@afrittoli afrittoli left a comment

Choose a reason for hiding this comment

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

/lgtm

@dibyom
Copy link
Copy Markdown
Member

dibyom commented Jan 27, 2021

/lgtm
Could we squash up the commits?

@tekton-robot tekton-robot removed the lgtm Indicates that a PR is ready to be merged. label Jan 27, 2021
@howardjohn
Copy link
Copy Markdown
Contributor Author

howardjohn commented Jan 27, 2021

@dibyom done

@vdemeester
Copy link
Copy Markdown
Member

vdemeester commented Jan 27, 2021

/lgtm

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 27, 2021
@dibyom
Copy link
Copy Markdown
Member

dibyom commented Jan 27, 2021

/hold cancel

@tekton-robot tekton-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 27, 2021
@dibyom dibyom added kind/misc Categorizes issue or PR as a miscellaneuous one. and removed kind/misc Categorizes issue or PR as a miscellaneuous one. labels Jan 27, 2021
@tekton-robot tekton-robot merged commit 060ec8e into tektoncd:master Jan 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vdemeester vdemeester vdemeester left review comments

@afrittoli afrittoli afrittoli left review comments

@pritidesai pritidesai Awaiting requested review from pritidesai

@mattmoor mattmoor Awaiting requested review from mattmoor

+1 more reviewer

@imjasonh imjasonh imjasonh approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@vdemeester vdemeester

@afrittoli afrittoli

@dibyom dibyom

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/misc Categorizes issue or PR as a miscellaneuous one. lgtm Indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@howardjohn @tekton-robot @vdemeester @dibyom @imjasonh @afrittoli