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.

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

Verify packageName in bundle matches repository name #49

Merged
merged 1 commit into from
Mar 19, 2019
Merged

Verify packageName in bundle matches repository name #49

merged 1 commit into from
Mar 19, 2019

Conversation

jeremy-wl
Copy link
Member

@jeremy-wl jeremy-wl commented Mar 8, 2019 •
edited
Loading

Verification step added to ensure the packageName defined in package.yaml matches the quay.io repo name given as a parameter to the push command.

@openshift-ci-robot openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Mar 8, 2019
@jeremy-wl jeremy-wl force-pushed the verify-package-name-in-bundle-match-quay-repo-name branch from cd0df55 to ff02378 Compare March 8, 2019 22:33
kevinrizza
kevinrizza requested changes Mar 8, 2019
View reviewed changes
operatorcourier/api.py Outdated Show resolved Hide resolved
operatorcourier/validate.py Show resolved Hide resolved
@jeremy-wl jeremy-wl force-pushed the verify-package-name-in-bundle-match-quay-repo-name branch from ff02378 to 1036b45 Compare March 11, 2019 16:06
@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 11, 2019
@jeremy-wl jeremy-wl force-pushed the verify-package-name-in-bundle-match-quay-repo-name branch from 1036b45 to 4b57e15 Compare March 11, 2019 19:51
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 11, 2019
@jeremy-wl
Copy link
Member Author

/cc @awgreene @kevinrizza

awgreene
awgreene requested changes Mar 12, 2019
View reviewed changes
Copy link
Member

@awgreene awgreene left a comment

Choose a reason for hiding this comment

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

Good work @jeremylinlin, please address my comments.

operatorcourier/validate.py Outdated Show resolved Hide resolved
@jeremy-wl jeremy-wl force-pushed the verify-package-name-in-bundle-match-quay-repo-name branch from 4b57e15 to 71e067d Compare March 12, 2019 16:07
@jeremy-wl
Copy link
Member Author

/cc @kevinrizza @awgreene

kevinrizza
kevinrizza requested changes Mar 13, 2019
View reviewed changes
operatorcourier/validate.py Outdated Show resolved Hide resolved
@jeremy-wl jeremy-wl force-pushed the verify-package-name-in-bundle-match-quay-repo-name branch from 71e067d to ae0c0d9 Compare March 13, 2019 14:59
@kevinrizza
Copy link
Member

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 13, 2019
SamiSousa
SamiSousa suggested changes Mar 15, 2019
View reviewed changes
Copy link
Contributor

@SamiSousa SamiSousa left a comment

Choose a reason for hiding this comment

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

One note: I know we don't want to introduce breaking changes, but is it possible to have a CLI flag that let's us skip entering the name operator bundle repo and instead uses the packageName found in the bundle?This is a nice-to-have, @kevinrizza let me know what you think.

operatorcourier/validate.py Outdated Show resolved Hide resolved
operatorcourier/validate.py Show resolved Hide resolved
@kevinrizza
Copy link
Member

One note: I know we don't want to introduce breaking changes, but is it possible to have a CLI flag that let's us skip entering the name operator bundle repo and instead uses the packageName found in the bundle?This is a nice-to-have, @kevinrizza let me know what you think.

@SamiSousa I don't want to do this because, while we technically have a soft requirement that packageName and repository name match, that's something that could be removed later. I would prefer not to introduce a feature that we may have to remove later.

@jeremy-wl jeremy-wl force-pushed the verify-package-name-in-bundle-match-quay-repo-name branch from ae0c0d9 to 9afcfa2 Compare March 15, 2019 19:15
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Mar 15, 2019
SamiSousa
SamiSousa approved these changes Mar 15, 2019
View reviewed changes
Copy link
Contributor

@SamiSousa SamiSousa 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 lgtm Indicates that a PR is ready to be merged. and removed lgtm Indicates that a PR is ready to be merged. labels Mar 15, 2019
@openshift-ci-robot
Copy link

New changes are detected. LGTM label has been removed.

@jeremy-wl jeremy-wl force-pushed the verify-package-name-in-bundle-match-quay-repo-name branch from 810a5a8 to 9afcfa2 Compare March 15, 2019 20:12
@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 15, 2019
@jeremy-wl jeremy-wl force-pushed the verify-package-name-in-bundle-match-quay-repo-name branch from 9afcfa2 to da5ca63 Compare March 15, 2019 20:16
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 15, 2019
@jeremy-wl
Verify packageName in bundle matches repository name
a3e0cb2 
- Check only 1 package is defined in the bundle
- Ensure the packageName package.yaml matches quay.io repo name
    given as a parameter to the push command
@jeremy-wl jeremy-wl force-pushed the verify-package-name-in-bundle-match-quay-repo-name branch from da5ca63 to a3e0cb2 Compare March 19, 2019 14:22
@kevinrizza kevinrizza merged commit a9f8785 into operator-framework:master Mar 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kevinrizza kevinrizza kevinrizza requested changes

@SamiSousa SamiSousa SamiSousa approved these changes

@gallettilance gallettilance gallettilance approved these changes

@MartinBasti MartinBasti Awaiting requested review from MartinBasti

@awgreene awgreene Awaiting requested review from awgreene

Assignees

@kevinrizza kevinrizza

@SamiSousa SamiSousa

Labels
size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@jeremy-wl @kevinrizza @openshift-ci-robot @awgreene @SamiSousa @gallettilance