Skip to content

Conversation

jhadvig
Copy link
Member

@jhadvig jhadvig commented Sep 22, 2020

Story: https://issues.redhat.com/browse/CONSOLE-2266

Will generate the CRD once we align on the api. PTAL

/assign @spadgett
cc'ing @rohitkrai03 @christianvogt

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.

Thanks, @jhadvig. I made some suggestions on the API doc descriptions.

@jhadvig jhadvig force-pushed the qs_crd branch 7 times, most recently from d0fe1d5 to 9a8eadf Compare September 24, 2020 11:56
@jhadvig
Copy link
Member Author

jhadvig commented Sep 24, 2020

@spadgett comments addressed. Added the validation as well + one comment. PTAL

@jhadvig
Copy link
Member Author

jhadvig commented Sep 24, 2020

@spadgett @christianvogt PR updated

@jhadvig
Copy link
Member Author

jhadvig commented Sep 29, 2020

@christianvogt @spadgett comments addressed

@spadgett
Copy link
Member

/assign @bparees
for API approval

/assign @ahardin-rh
for docs approval
@ahardin-rh This will have doc impact for 4.7.

@spadgett spadgett changed the title Add Console QuickStart CRD CONSOLE-2266: Add Console QuickStart CRD Oct 13, 2020
Copy link
Contributor

@bparees bparees left a comment

Choose a reason for hiding this comment

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

the "console" prefix seems unnecessary/redundant in most of the places it is used, including the CRD name itself.

@jhadvig
Copy link
Member Author

jhadvig commented Oct 14, 2020

@bparees comment addressed :)

@jhadvig
Copy link
Member Author

jhadvig commented Oct 27, 2020

@soltysh comment addressed.
@spadgett @bparees FYI

Copy link
Contributor

@soltysh soltysh left a comment

Choose a reason for hiding this comment

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

lgtm, but I'm leaving final tags to Ben and Sam

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 27, 2020
@bparees
Copy link
Contributor

bparees commented Oct 27, 2020

@jhadvig i still see numerous unresolved comment threads, please reply to them and/or mark them resolved (if they are)

@jhadvig
Copy link
Member Author

jhadvig commented Oct 28, 2020

@bparees all comments resolved and PR updated.
@spadgett FYI

@andrewballantyne
Copy link

@jhadvig @spadgett @bparees Is there anything left that is blocking this from merging?

cc @christianvogt @rohitkrai03

@jhadvig
Copy link
Member Author

jhadvig commented Nov 10, 2020

@andrewballantyne there should not be anything, based on the Slack conversations. I've resolved all the comments and the PR should be ready to get merged.

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.

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 10, 2020
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jhadvig, soltysh, spadgett

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:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-robot openshift-merge-robot merged commit 31aac52 into openshift:master Nov 10, 2020
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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants