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

Add --operator-name flag to olm-catalog gen-csv #1571

Merged

Conversation

nikhil-thomas
Copy link
Contributor

@nikhil-thomas nikhil-thomas commented Jun 19, 2019

Description of the change:
This patch adds a flag --operator-name to specify the operator name
while generating CSV using operator-sdk gen-csv command

The default value (when flag is absent) is base directory name as before

Motivation for the change:
At present operator-sdk assumes project-directory-name (ProjectName) is the operator name.
The operator-name and the project-directory-name can be different.
(because of change in source control, project home, organizational policies etc)

Without an --operator-name flag we will have to rename, copy-paste generated CSVs

Signed-off-by: Nikhil Thomas nikthoma@redhat.com

@openshift-ci-robot openshift-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jun 19, 2019
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.

There are a few other changes required to use a different CSV name and path simultaneously in a reliable manner.

An OperatorName field should be added to scaffold.CSV so input.ProjectName can be used elsewhere as expected. OperatorName should be populated as ProjectName is currently, and used instead of ProjectName in internal/pkg/scaffold/olm-catalog/csv.go where relevant. The --project-name flag should be renamed to --operator-name.

@joelanford
Copy link
Member

Are there currently (or do we envision in the future) any other subcommands that would need a similar flag? I wonder if we should add this flag to the operator-sdk new command instead, and populate a config file in the project root that contains the value, then any other commands that need it could just pull it from there?

@estroz
Copy link
Member

estroz commented Jun 19, 2019

@joelanford yes especially with OLM additions coming. There are quite a few changes to be made before we have a global config, but we can add it to the CSV config for now as operator-name. Other commands can access it that way.

@openshift-ci-robot openshift-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jun 19, 2019
@nikhil-thomas
Copy link
Contributor Author

@estroz Thank you for the feedback 👍

I have updated my patch based on your review. Could take a look.

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.

Needs changes.

cmd/operator-sdk/olmcatalog/gen-csv.go Outdated Show resolved Hide resolved
internal/pkg/scaffold/input/input.go Outdated Show resolved Hide resolved
internal/pkg/scaffold/input/input.go Outdated Show resolved Hide resolved
internal/pkg/scaffold/input/input.go Outdated Show resolved Hide resolved
internal/pkg/scaffold/olm-catalog/csv.go Show resolved Hide resolved
internal/pkg/scaffold/olm-catalog/csv.go Show resolved Hide resolved
internal/pkg/scaffold/olm-catalog/csv.go Show resolved Hide resolved
internal/pkg/scaffold/scaffold.go Outdated Show resolved Hide resolved
internal/pkg/scaffold/scaffold.go Outdated Show resolved Hide resolved
internal/pkg/scaffold/scaffold.go Outdated Show resolved Hide resolved
@nikhil-thomas nikhil-thomas changed the title Add --project-name flag to olmcatalog gen-csv Add --operator-name flag to olmcatalog gen-csv Jun 20, 2019
This pactch adds a `--operator-name` flag to `operator-sdk olm-catalog gen-csv` command.
At present operator-sdk  assumes project-directory-name (ProjectName) is the operator name.
The operator-name and the project-directory-name can be different.
(because of change in source control, project home, organizational policies etc)

Without an --operator-name flag we will have to rename, copy-paste generated CSVs

Signed-off-by: Nikhil Thomas <nikthoma@redhat.com>
@nikhil-thomas nikhil-thomas changed the title Add --operator-name flag to olmcatalog gen-csv Add --operator-name flag to olm-catalog gen-csv Jun 20, 2019
@nikhil-thomas
Copy link
Contributor Author

@estroz Thank you for the detailed review. I have made the changes.
Could you take a look.

@lilic lilic requested a review from estroz June 28, 2019 14:02
@lilic
Copy link
Member

lilic commented Jun 28, 2019

cc @estroz think this is ready for another look.

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.

/lgtm

@estroz
Copy link
Member

estroz commented Jun 28, 2019

/cc @joelanford

I'll make a follow-up PR for a operator-name config field.

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.

/approve

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants