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

Remove legacy generate and bundle commands #3414

Merged
merged 2 commits into from
Jul 17, 2020

Conversation

hasbro17
Copy link
Contributor

@hasbro17 hasbro17 commented Jul 13, 2020

Description of the change:
Removed the following legacy cmds:

  • generate csv
  • generate packagemanifests
  • generate bundle
  • bundle create
  • bundle validate

Also removed associated scaffolding packages for these commands in internal/generate/olm-catalog.

Motivation for the change:

From #3327 : Prepare SDK CLI for 1.0.0 release

TODO:

@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 Jul 13, 2020
@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 13, 2020
@joelanford joelanford mentioned this pull request Jul 13, 2020
92 tasks
@@ -51,7 +51,6 @@ More information about the integration with OLM via SDK:
https://sdk.operatorframework.io/docs/olm-integration/legacy
`
cmd.AddCommand(
Copy link
Member

Choose a reason for hiding this comment

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

You can remove the entire bundle subcommand for legacy projects.

@@ -1,261 +0,0 @@
// Copyright 2019 The Operator-SDK Authors
Copy link
Member

Choose a reason for hiding this comment

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

This package still needs to stick around for the new CSV bases to work. #2792 should be updated to remove these. You can just leave this package in internal/generate/olm-catalog/descriptor for now.

@hasbro17 hasbro17 force-pushed the remove-gen-csv branch 2 times, most recently from 22108bb to ad78b43 Compare July 16, 2020 20:06
@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 16, 2020
@hasbro17 hasbro17 changed the title WIP: Remove legacy generate csv/pm/bundle and bundle create Remove legacy generate and bundle commands Jul 16, 2020
@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 16, 2020
Removed legacy "generate csv/packagemanifests/bundle", and "bundle create/validate" commands.
Also removed associated scaffolding packages for these commands in "internal/generate/olm-catalog".
Copy link
Member

@estroz estroz left a comment

Choose a reason for hiding this comment

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

Two nits about refactoring functions and fix lint errors about unused fields, otherwise LGTM

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
It shows great for me 👍
I cannot see anything else that is missing get done here.

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

@varshaprasad96 varshaprasad96 left a comment

Choose a reason for hiding this comment

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

Checked out by running with these changes locally.
/lgtm

@hasbro17 hasbro17 merged commit 76b641d into operator-framework:master Jul 17, 2020
@hasbro17 hasbro17 deleted the remove-gen-csv branch July 17, 2020 01:09
@hasbro17
Copy link
Contributor Author

Thanks for the quick reviews everyone.

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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants