Skip to content

<run|cleanup> packagemanifests: remove --include-paths, change --operator-version to --version #3599

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
merged 1 commit into from
Jul 29, 2020
Merged

<run|cleanup> packagemanifests: remove --include-paths, change --operator-version to --version #3599

merged 1 commit into from
Jul 29, 2020

Conversation

estroz
Copy link
Member

@estroz estroz commented Jul 29, 2020

Description of the change:

  • internal/olm/operator: remove include paths and change operator version to version
  • website: update docs

Motivation for the change: streamline flags

/cc @joelanford @rashmigottipati @jmrodri

Checklist

If the pull request includes user-facing changes, extra documentation is required:

@estroz estroz added the olm-integration Issue relates to the OLM integration label Jul 29, 2020
@estroz
<run|cleanup> packagemanifests flag --include-paths has been remo…
905a829 
…ved,

and `--operator-version` has been changed to `--version`
@estroz estroz force-pushed the chore/change-run-flags branch from a466a2a to 905a829 Compare July 29, 2020 18:56
camilamacedo86
camilamacedo86 approved these changes Jul 29, 2020
View reviewed changes
Copy link
Contributor

@camilamacedo86 camilamacedo86 left a comment

Choose a reason for hiding this comment

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

/lgtm

when pass in the Ci

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 29, 2020
joelanford
joelanford approved these changes Jul 29, 2020
View reviewed changes
Copy link
Member

@joelanford joelanford left a comment

Choose a reason for hiding this comment

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

/lgtm

rashmigottipati
rashmigottipati reviewed Jul 29, 2020
View reviewed changes
Copy link
Member

@rashmigottipati rashmigottipati left a comment

Choose a reason for hiding this comment

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

Why are we removing the checks hasSubscription and hasCatalogSource?

@estroz
Copy link
Member Author

estroz commented Jul 29, 2020

Why are we removing the checks hasSubscription and hasCatalogSource?

There is no way for a user to pass a Subscription or CatalogSource to the run packagemanifests command, so these cannot exist in the list of objects to create (the removed operatorManager.olmObjects) before the SDK constructs them internally. Therefore these checks are unnecessary.

rashmigottipati
rashmigottipati approved these changes Jul 29, 2020
View reviewed changes
Copy link
Member

@rashmigottipati rashmigottipati left a comment

Choose a reason for hiding this comment

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

/lgtm

@estroz estroz merged commit e5d23f0 into operator-framework:master Jul 29, 2020
@estroz estroz deleted the chore/change-run-flags branch July 29, 2020 19:38
@joelanford joelanford mentioned this pull request Jul 29, 2020
Closed
92 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joelanford joelanford joelanford approved these changes

@rashmigottipati rashmigottipati rashmigottipati approved these changes

@camilamacedo86 camilamacedo86 camilamacedo86 approved these changes

@jmrodri jmrodri Awaiting requested review from jmrodri

Assignees

@joelanford joelanford

@rashmigottipati rashmigottipati

@camilamacedo86 camilamacedo86

Labels
lgtm Indicates that a PR is ready to be merged. olm-integration Issue relates to the OLM integration
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@estroz @joelanford @rashmigottipati @camilamacedo86 @openshift-ci-robot