Skip to content

Add console extension manfiest for ConsoleExternalLogLinks CRD #246

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
openshift-merge-robot merged 1 commit into from
Jul 20, 2019
Merged

Add console extension manfiest for ConsoleExternalLogLinks CRD #246

openshift-merge-robot merged 1 commit into from
Jul 20, 2019

Conversation

@rhamilto
Copy link
Member

@rhamilto rhamilto commented Jun 24, 2019 •
edited
Loading

@openshift-ci-robot openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jun 24, 2019
spadgett
spadgett reviewed Jun 24, 2019
View reviewed changes
@rhamilto rhamilto changed the title Add console extension manfiest for ConsolePodLogLinks CRD [WIP] Add console extension manfiest for ConsolePodLogLinks CRD Jun 24, 2019
@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 24, 2019
benjaminapetersen
benjaminapetersen reviewed Jun 24, 2019
View reviewed changes
@benjaminapetersen
Copy link
Contributor

benjaminapetersen commented Jun 25, 2019

/hold

Note that ci/prow/e2e-aws will fail unless we bump the rbac test again.

Since we are holding off on this PR for design, We will likely have the CRD proxy for the console server done by then, so shouldn't need to do this.

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 25, 2019
@rhamilto rhamilto changed the title [WIP] Add console extension manfiest for ConsolePodLogLinks CRD [WIP] Add console extension manfiest for ConsoleLogLinks CRD Jul 2, 2019
@rhamilto rhamilto force-pushed the console-1528 branch 2 times, most recently from 973117f to 2d15884 Compare July 5, 2019 16:49
@rhamilto rhamilto changed the title [WIP] Add console extension manfiest for ConsoleLogLinks CRD Add console extension manfiest for ConsoleLogLinks CRD Jul 5, 2019
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 5, 2019
@rhamilto
Copy link
Member Author

rhamilto commented Jul 5, 2019

Updated. @spadgett, PTAL.

@rhamilto
Copy link
Member Author

rhamilto commented Jul 8, 2019

/retest

1 similar comment
@jhadvig
Copy link
Member

jhadvig commented Jul 15, 2019

/retest

@benjaminapetersen
Copy link
Contributor

benjaminapetersen commented Jul 15, 2019

Needs an update like this one for the tests to pass:
openshift/origin#23231
Working on that now.

@benjaminapetersen benjaminapetersen mentioned this pull request Jul 15, 2019
Merged
@jhadvig jhadvig mentioned this pull request Jul 16, 2019
Merged
benjaminapetersen
benjaminapetersen reviewed Jul 17, 2019
View reviewed changes
benjaminapetersen
benjaminapetersen reviewed Jul 17, 2019
View reviewed changes
@rhamilto rhamilto force-pushed the console-1528 branch 2 times, most recently from ef77196 to 9419bbf Compare July 17, 2019 22:15
@spadgett
Copy link
Member

spadgett commented Jul 18, 2019

@rhamilto we should match the descriptions here to what's in openshift/api since it's what's added to the OpenAPI doc (oc explain, API Explorer, etc.)

Note there is one minor change: openshift/api#385

benjaminapetersen
benjaminapetersen reviewed Jul 18, 2019
View reviewed changes
@benjaminapetersen
Copy link
Contributor

benjaminapetersen commented Jul 18, 2019

@rhamilto want to retitle PR for ConsoleExternalLogLink? (just for future reference parity)

@rhamilto
Copy link
Member Author

rhamilto commented Jul 18, 2019

Descriptions updated to align with api. Note I had to make some formatting adjustments to make the text yaml-friendly (removed colon after Supported variables are and changed - bullet to *.
@spadgett, @benjaminapetersen

@rhamilto rhamilto changed the title Add console extension manfiest for ConsoleLogLinks CRD Add console extension manfiest for ConsoleExternalLogLinks CRD Jul 18, 2019
spadgett
spadgett approved these changes Jul 18, 2019
View reviewed changes
Copy link
Member

@spadgett spadgett left a comment

Choose a reason for hiding this comment

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

/approve
/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 18, 2019
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Jul 18, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rhamilto, spadgett

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

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 18, 2019
@spadgett
Copy link
Member

spadgett commented Jul 18, 2019

Feel free to remove the hold when openshift/origin#23382 lands

@rhamilto
Copy link
Member Author

rhamilto commented Jul 19, 2019

/retest

@spadgett
Copy link
Member

spadgett commented Jul 20, 2019

/retest
/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 20, 2019
@spadgett
Copy link
Member

spadgett commented Jul 20, 2019

/retest

@openshift-merge-robot openshift-merge-robot merged commit 9963238 into openshift:master Jul 20, 2019
@rhamilto rhamilto deleted the console-1528 branch July 22, 2019 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@spadgett spadgett spadgett approved these changes

@jhadvig jhadvig Awaiting requested review from jhadvig

+1 more reviewer

@benjaminapetersen benjaminapetersen benjaminapetersen left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@benjaminapetersen benjaminapetersen

@spadgett spadgett

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. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@rhamilto @benjaminapetersen @jhadvig @spadgett @openshift-ci-robot @openshift-merge-robot