Skip to content

Migrate pkg to istio.io/client-go apis #882

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
knative-prow-robot merged 3 commits into from
Nov 26, 2019
Merged

Migrate pkg to istio.io/client-go apis #882

knative-prow-robot merged 3 commits into from
Nov 26, 2019

Conversation

@skaslev
Copy link

@skaslev skaslev commented Nov 15, 2019 •
edited
Loading

Migrate pkg to istio.io/client-go apis.

This is preliminary work towards fixing knative/serving#5969 and knative/serving#5936.

@googlebot googlebot added the cla: yes Indicates the PR's author has signed the CLA. label Nov 15, 2019
@knative-prow-robot knative-prow-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. area/test-and-release needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Nov 15, 2019
@knative-prow-robot
Copy link
Contributor

knative-prow-robot commented Nov 15, 2019

Hi @skaslev. Thanks for your PR.

I'm waiting for a knative 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.

@skaslev skaslev mentioned this pull request Nov 15, 2019
Merged
@skaslev skaslev changed the title Migrate to istio.io/client-go apis Migrate pkg to istio.io/client-go apis Nov 15, 2019
@tcnghia
Copy link
Contributor

tcnghia commented Nov 15, 2019

/ok-to-test

@knative-prow-robot knative-prow-robot added 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 Nov 15, 2019
@tcnghia
Copy link
Contributor

tcnghia commented Nov 15, 2019

@skaslev looks like there are merge conflicts.

markusthoemmes
markusthoemmes reviewed Nov 20, 2019
View reviewed changes
@skaslev skaslev mentioned this pull request Nov 20, 2019
Closed
3 tasks
@coryrc
Copy link
Contributor

coryrc commented Nov 20, 2019

/lgtm

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 20, 2019
@coryrc
Copy link
Contributor

coryrc commented Nov 21, 2019 •
edited
Loading

Neither Chi nor I are appropriate reviewers for this. I'll Victor, it it's not him he'll at least know...
/uncc @coryrc
/uncc @chizhg
/cc @vagababov

(argh, I wish github would warn you when the page isn't updated -- obviously Markus can review)

@knative-prow-robot knative-prow-robot requested review from vagababov and removed request for chizhg and coryrc November 21, 2019 01:27
@vagababov
Copy link
Contributor

vagababov commented Nov 21, 2019

/assign @tcnghia

@tcnghia
Copy link
Contributor

tcnghia commented Nov 26, 2019

@skaslev Currently there is no test for this, can you please try out if our integration tests in Knative/serving would pass with this change, using a WIP change? Thanks

@skaslev
Copy link
Author

skaslev commented Nov 26, 2019 •
edited
Loading

@skaslev Currently there is no test for this, can you please try out if our integration tests in Knative/serving would pass with this change, using a WIP change? Thanks

I've already done it in this PR knative/serving#6041 all serving tests are passing.

The PRs need to be rebased because of merge conflicts though. I'll do that and have them do another run of the tests.

@knative-prow-robot knative-prow-robot removed the lgtm Indicates that a PR is ready to be merged. label Nov 26, 2019
@tcnghia
Copy link
Contributor

tcnghia commented Nov 26, 2019

/lgtm
/approve

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 26, 2019
@skaslev
Copy link
Author

skaslev commented Nov 26, 2019

/assign @vaikas

@tcnghia
Copy link
Contributor

tcnghia commented Nov 26, 2019

@mattmoor can you please /approve ?

@mattmoor
Copy link
Member

mattmoor commented Nov 26, 2019

Do we have a PR that stages this into knative/serving so that we can land these in quick succession? This will be breaking until that's fixed.

@skaslev
Copy link
Author

skaslev commented Nov 26, 2019

There's knative/serving#6041 which has knative.dev/pkg pinned to my clone for testing but can quickly be switched back after this land.

@mattmoor
Copy link
Member

mattmoor commented Nov 26, 2019

/approve
/hold

You can use /hold cancel when you are ready to merge this and stage the other. Feel free to coordinate with me for getting the other PR merged once this lands.

@knative-prow-robot knative-prow-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 26, 2019
@knative-prow-robot
Copy link
Contributor

knative-prow-robot commented Nov 26, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mattmoor, skaslev, tcnghia

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

@knative-prow-robot knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 26, 2019
@mattmoor
Copy link
Member

mattmoor commented Nov 26, 2019

/hold cancel

@knative-prow-robot knative-prow-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 26, 2019
@knative-prow-robot knative-prow-robot merged commit ddf3968 into knative:master Nov 26, 2019
@skaslev skaslev deleted the istio-apis2 branch November 28, 2019 09:03
@skaslev skaslev mentioned this pull request Dec 4, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vagababov vagababov Awaiting requested review from vagababov

1 more reviewer

@markusthoemmes markusthoemmes markusthoemmes left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@tcnghia tcnghia

@coryrc coryrc

@vaikas vaikas

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. area/test-and-release cla: yes Indicates the PR's author has signed the CLA. 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. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@skaslev @knative-prow-robot @tcnghia @coryrc @vagababov @mattmoor @markusthoemmes @googlebot @vaikas