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.

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

Switch to openshift/api #17477

Merged
merged 14 commits into from Nov 28, 2017
Merged

Switch to openshift/api #17477

merged 14 commits into from Nov 28, 2017

Conversation

deads2k
Copy link
Contributor

@deads2k deads2k commented Nov 27, 2017

Starts vendoring github.com/openshift/api instead of using the native types. I haven't updated unit tests yet, but I could use some CI runs to chase problems.

@openshift-ci-robot openshift-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Nov 27, 2017
@openshift-merge-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k

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

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@openshift-merge-robot openshift-merge-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. needs-api-review labels Nov 27, 2017
@deads2k deads2k force-pushed the api-12-api branch 6 times, most recently from 69b85b9 to 9d70924 Compare November 27, 2017 21:26
@deads2k
Copy link
Contributor Author

deads2k commented Nov 27, 2017

@liggitt This is actually pretty close. I'm chasing a failure in test/cmd/image, but that's the last one I know of.

@deads2k
Copy link
Contributor Author

deads2k commented Nov 27, 2017

Oh, and we'll fix the proto definition stuff later. No idea why it dropped out.

@deads2k
Copy link
Contributor Author

deads2k commented Nov 27, 2017

@deads2k
Copy link
Contributor Author

deads2k commented Nov 27, 2017

/test extended_builds

@deads2k
Copy link
Contributor Author

deads2k commented Nov 27, 2017

@mfojtik I'm likely to get this ready tomorrow am. ptal

@mfojtik
Copy link
Member

mfojtik commented Nov 28, 2017

@deads2k i'm going to try to chase the cmd failure till you get in.

@@ -1,4 +1,2 @@
// +k8s:deepcopy-gen=package,register
Copy link
Member

Choose a reason for hiding this comment

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

why you dropped this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

why you dropped this?

it is generated in openshift/api.

// +k8s:defaulter-gen=TypeMeta
// +k8s:openapi-gen=true
// +k8s:defaulter-gen-input=../../../../../../../../github.com/openshift/api/network/v1
Copy link
Member

Choose a reason for hiding this comment

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

this is pretty ugly... can we improve the generator?

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 pretty ugly... can we improve the generator?

Yeah. This one in particular is off. Once we break the back of the problem we can fine tune.

@mfojtik
Copy link
Member

mfojtik commented Nov 28, 2017

Can't repro this locally with this PR rebased against master, so I guess you just need a rebase and we are good to go.

/retest

@mfojtik
Copy link
Member

mfojtik commented Nov 28, 2017

@deads2k seems like hack/verify-generated-swagger-descriptions.sh is gone

@deads2k
Copy link
Contributor Author

deads2k commented Nov 28, 2017

@deads2k seems like hack/verify-generated-swagger-descriptions.sh is gone

Yeah. Also done in openshift/api

@deads2k
Copy link
Contributor Author

deads2k commented Nov 28, 2017

Got an lgtm via irc. tagging and bumping priority.

@deads2k deads2k added lgtm Indicates that a PR is ready to be merged. queue/critical-fix labels Nov 28, 2017
@deads2k
Copy link
Contributor Author

deads2k commented Nov 28, 2017

/retest

@openshift-merge-robot
Copy link
Contributor

/test all [submit-queue is verifying that this PR is safe to merge]

@deads2k
Copy link
Contributor Author

deads2k commented Nov 28, 2017

/retest

@deads2k
Copy link
Contributor Author

deads2k commented Nov 28, 2017

/retest

1 similar comment
@deads2k
Copy link
Contributor Author

deads2k commented Nov 28, 2017

/retest

@liggitt
Copy link
Contributor

liggitt commented Nov 28, 2017

all blocking CI jobs were green on the PR HEAD commit
PR is rebased on current master
manually merging to unblock other PRs this would conflict with

@liggitt liggitt merged commit 58c9ab7 into openshift:master Nov 28, 2017
@openshift-ci-robot
Copy link

@deads2k: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
ci/openshift-jenkins/extended_conformance_install_update 6b0e2c4 link /test extended_conformance_install_update
ci/openshift-jenkins/extended_conformance_install 6b0e2c4 link /test extended_conformance_install

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

@deads2k deads2k deleted the api-12-api branch January 24, 2018 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. needs-api-review queue/critical-fix size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants