Skip to content
This repository was archived by the owner on Sep 21, 2020. It is now read-only.

Fix OLM Steps for Getting Started #57

Merged
merged 3 commits into from
Nov 5, 2019
Merged

Fix OLM Steps for Getting Started #57

merged 3 commits into from
Nov 5, 2019

Conversation

camilamacedo86
Copy link
Contributor

@camilamacedo86 camilamacedo86 commented Oct 23, 2019 •
edited
Loading

Description

  • Remove outdated/duplicated info regards the steps to integrate with OLM and how to test it locally
  • Add notes in order to make clear the purpose of OLM as let the users check other examples and possibilities. E.g use the operatorhub.io to create the bundle and/or validate/preview the CSV file.

Motivation

(Regards CSV/OLM)

@openshift-ci-robot openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 23, 2019
@camilamacedo86 camilamacedo86 mentioned this pull request Oct 24, 2019
Closed
camilamacedo86
camilamacedo86 commented Oct 25, 2019
View reviewed changes
camilamacedo86
camilamacedo86 commented Oct 25, 2019
View reviewed changes
camilamacedo86
camilamacedo86 commented Oct 25, 2019
View reviewed changes
@camilamacedo86 camilamacedo86 changed the title Update Getting Started in order to keep its updated Fix Getting Started Oct 25, 2019
@camilamacedo86
Copy link
Contributor Author

Hi @joelanford,

It has the changes that we spoke about. Could we move forward in this way?

@camilamacedo86 camilamacedo86 changed the title Fix Getting Started Fix OLM Steps for Getting Started Oct 27, 2019
dmesser
dmesser reviewed Oct 28, 2019
View reviewed changes
dmesser
dmesser reviewed Oct 28, 2019
View reviewed changes
@dmesser
Copy link
Contributor

dmesser commented Oct 28, 2019

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 28, 2019
@camilamacedo86
Copy link
Contributor Author

Hi @joelanford,

@dmesser give his /lgtm here. So, I am looking for your ok too for we are able to merge it. Could you please let us know if we can merge this one?

estroz
estroz suggested changes Oct 31, 2019
View reviewed changes
README.md Outdated

Because OLM creates operators in a particular namespace when a manifest has been applied, administrators can leverage the native Kubernetes RBAC permission model to restrict which users are allowed to install operators.
**NOTE:** Also, you may would like to check a feature to build the bundle in the [OperatorHub.io][operator-hub-i-bundle].
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to make note of this, or direct users to only use gen-csv? @dmesser

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that would be nice to give the opportunity/info in order to allow users to check this feature as well.

@openshift-ci-robot
Copy link

New changes are detected. LGTM label has been removed.

@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Oct 31, 2019
@camilamacedo86 @dmesser
update OLM steps to help users
de70e62 
Co-Authored-By: Daniel Messer <[email protected]>
@camilamacedo86 camilamacedo86 requested a review from estroz October 31, 2019 20:35
@camilamacedo86
Copy link
Contributor Author

Hi @estroz,

99% of your suggestions are applied. Just one as explained in the comment was not. Please, let us know if you /lgtm /approve this one now. Also, @dmesser already /lgtm /approve this one before but to merge I will wait for his second view and let us know if he s ok or not with the NOTE about the bundle feature in OperatorHub.io.

estroz
estroz approved these changes Nov 1, 2019
View reviewed changes
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

after addressing two nits.

@camilamacedo86
Update README.md
2e7c789 
Co-Authored-By: Eric Stroczynski <[email protected]>
camilamacedo86
camilamacedo86 commented Nov 1, 2019
View reviewed changes
@camilamacedo86 camilamacedo86 added 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. labels Nov 1, 2019
@dmesser
Copy link
Contributor

dmesser commented Nov 5, 2019

/lgtm

@camilamacedo86
Copy link
Contributor Author

camilamacedo86 commented Nov 5, 2019 •
edited
Loading

Hi @estroz the @dmesser,

Checked it again and /lgtm for the second time. So, since all were addressed and it has 2 LGTM I am moving forward by merging this one.

@camilamacedo86 camilamacedo86 merged commit cbca033 into operator-framework:master Nov 5, 2019
@camilamacedo86 camilamacedo86 deleted the update-fix branch November 5, 2019 18:17
This was referenced Nov 5, 2019
Closed
Closed
Closed
Closed
@adellape adellape mentioned this pull request Mar 7, 2020
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@estroz estroz estroz approved these changes

@hasbro17 hasbro17 Awaiting requested review from hasbro17

@jmrodri jmrodri Awaiting requested review from jmrodri

@joelanford joelanford Awaiting requested review from joelanford

@asmacdo asmacdo Awaiting requested review from asmacdo

@jmccormick2001 jmccormick2001 Awaiting requested review from jmccormick2001

@ecordell ecordell Awaiting requested review from ecordell

@dmesser dmesser Awaiting requested review from dmesser

Assignees

@dmesser dmesser

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. 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.
4 participants
@camilamacedo86 @dmesser @openshift-ci-robot @estroz