Skip to content

Conversation

estroz
Copy link
Member

@estroz estroz commented Jul 17, 2019

Description of the change:

  • internal/pkg/scaffold/olm-catalog: preserve spec.customresourcedefinitions.owned elements and use unique ID's for accessing each owned CRD
  • internal/util/k8sutil: more robust GetTypeMetaFromBytes() instead of
    GetKindFromYAML()

Motivation for the change: users that define a spec.customresourcedefinitions.owned element currently have to enter data manually (until #1162 is merged). Manually entered data should be preserved for all spec.customresourcedefinition.owneds that already exist; newly added CRD's will be appended and modified by users.

…tions

fields and use unique ID's for each owned CRD

internal/util/k8sutil: more robust GetTypeMetaFromBytes instead of
GetKindFromYAML
@estroz estroz added the olm-integration Issue relates to the OLM integration label Jul 17, 2019
@openshift-ci-robot openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jul 17, 2019
@estroz
Copy link
Member Author

estroz commented Jul 17, 2019

/test marker

@estroz
Copy link
Member Author

estroz commented Jul 17, 2019

/test e2e-aws-ansible

@estroz
Copy link
Member Author

estroz commented Jul 17, 2019

/test e2e-aws-helm
/test e2e-aws-ansible
/test images

1 similar comment
@estroz
Copy link
Member Author

estroz commented Jul 17, 2019

/test e2e-aws-helm
/test e2e-aws-ansible
/test images

Copy link
Contributor

@hasbro17 hasbro17 left a comment

Choose a reason for hiding this comment

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

LGTM after nits.

Co-Authored-By: Haseeb Tariq <hasbro17@gmail.com>
@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 22, 2019
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 22, 2019
@estroz
Copy link
Member Author

estroz commented Jul 22, 2019

/test e2e-aws-helm
/test e2e-aws-ansible
/test e2e-aws-go

2 similar comments
@estroz
Copy link
Member Author

estroz commented Jul 22, 2019

/test e2e-aws-helm
/test e2e-aws-ansible
/test e2e-aws-go

@estroz
Copy link
Member Author

estroz commented Jul 23, 2019

/test e2e-aws-helm
/test e2e-aws-ansible
/test e2e-aws-go

Copy link
Member

@jmrodri jmrodri left a comment

Choose a reason for hiding this comment

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

really clean looking. And no typos either.

@jmrodri
Copy link
Member

jmrodri commented Jul 26, 2019

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 26, 2019
@estroz estroz merged commit 4b6d64b into operator-framework:master Aug 5, 2019
@estroz estroz deleted the csv-updates branch August 5, 2019 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm Indicates that a PR is ready to be merged. olm-integration Issue relates to the OLM integration size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants