Skip to content

Adding equivalent opm index flags to opm registry #827

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
Nov 15, 2021
Merged

Adding equivalent opm index flags to opm registry #827

merged 1 commit into from
Nov 15, 2021

Conversation

@gallettilance
Copy link
Member

@gallettilance gallettilance commented Nov 15, 2021

Description of the change:

Motivation for the change:

Reviewer Checklist

  • Implementation matches the proposed design, or proposal is updated to match implementation
  • Sufficient unit test coverage
  • Sufficient end-to-end test coverage
  • Docs updated or added to /docs
  • Commit messages sensible and descriptive

@openshift-ci openshift-ci bot requested review from ecordell and njhale November 15, 2021 15:22
@gallettilance
Adding equivalent opm index flags to opm registry
1c06380 
Signed-off-by: Lance Galletti <galletti@bu.edu>
@codecov
Copy link

codecov bot commented Nov 15, 2021 •
edited
Loading

Codecov Report

Merging #827 (1c06380) into master (7a789fe) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #827   +/-   ##
=======================================
  Coverage   51.46%   51.46%           
=======================================
  Files         103      103           
  Lines        9032     9032           
=======================================
  Hits         4648     4648           
  Misses       3510     3510           
  Partials      874      874

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7a789fe...1c06380. Read the comment docs.

camilamacedo86
camilamacedo86 reviewed Nov 15, 2021
View reviewed changes
cmd/opm/registry/add.go
enableAlpha, err := cmd.Flags().GetBool("enable-alpha")
if err != nil {
return err
}
Copy link
Contributor

@camilamacedo86 camilamacedo86 Nov 15, 2021

Choose a reason for hiding this comment

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

That shows fine for me. However, how can we ensure that the options will work well as well?
Should we not add tests for those?

Copy link
Member Author

@gallettilance gallettilance Nov 15, 2021

Choose a reason for hiding this comment

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

all the tests already exist here and here- this is just about exposing the flags

anik120
anik120 approved these changes Nov 15, 2021
View reviewed changes
Copy link
Member

@anik120 anik120 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 openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 15, 2021
@kevinrizza
Copy link
Member

kevinrizza commented Nov 15, 2021

/approve

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 15, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: anik120, gallettilance, kevinrizza

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 15, 2021
@openshift-merge-robot openshift-merge-robot merged commit b95ebb1 into operator-framework:master Nov 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@camilamacedo86 camilamacedo86 camilamacedo86 left review comments

@anik120 anik120 anik120 approved these changes

@ecordell ecordell Awaiting requested review from ecordell

@njhale njhale Awaiting requested review from njhale

Assignees

@anik120 anik120

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@gallettilance @kevinrizza @anik120 @camilamacedo86 @openshift-merge-robot