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

Update downstream CSV description to link to official doc #397

Merged
merged 3 commits into from
Jul 20, 2023

Conversation

jotak
Copy link
Member

@jotak jotak commented Jul 18, 2023

This is a small update to the CSV description used for downstream, that we should probably have done already earlier: removes the mentions of our unofficial guides, replaced with a link to the official doc

@openshift-ci
Copy link

openshift-ci bot commented Jul 18, 2023

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from jotak. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found 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

@codecov
Copy link

codecov bot commented Jul 18, 2023

Codecov Report

Merging #397 (035423e) into main (a4a3c26) will decrease coverage by 0.85%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #397      +/-   ##
==========================================
- Coverage   54.52%   53.67%   -0.85%     
==========================================
  Files          44       44              
  Lines        5423     5559     +136     
==========================================
+ Hits         2957     2984      +27     
- Misses       2259     2359     +100     
- Partials      207      216       +9     
Flag Coverage Δ
unittests 53.67% <ø> (-0.85%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 9 files with indirect coverage changes

@@ -8,13 +8,9 @@ The operator provides dashboards, metrics, and keeps flows accessible in a query

[Loki](https://grafana.com/oss/loki/), from GrafanaLabs, is the backend that is used to store all collected flows. The NetObserv Operator does not install Loki directly, however we provide some guidance to help you there.

For normal usage, we recommend two options:
- For a persistent usage, refer to [the operator documentation](https://docs.openshift.com/container-platform/latest/networking/network_observability/installing-operators.html).
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- For a persistent usage, refer to [the operator documentation](https://docs.openshift.com/container-platform/latest/networking/network_observability/installing-operators.html).
- For production usage, refer to [the operator documentation](https://docs.openshift.com/container-platform/latest/networking/network_observability/installing-operators.html).

Copy link
Contributor

Choose a reason for hiding this comment

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

+1

Copy link
Member Author

Choose a reason for hiding this comment

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

ok - I was thinking it's more than "production", for instance a persistent staging environment should also refer to the doc - any other thoughts then?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmmm perhaps "a production or production-like test environment" ?

Copy link
Member Author

Choose a reason for hiding this comment

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

@skrthomas , wdyt, sounds good to you?

Copy link
Contributor

Choose a reason for hiding this comment

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

That suggestion is good with me!

Copy link
Contributor

@skrthomas skrthomas left a comment

Choose a reason for hiding this comment

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

/lgtm thanks Joel!

@openshift-ci
Copy link

openshift-ci bot commented Jul 19, 2023

New changes are detected. LGTM label has been removed.

@openshift-ci openshift-ci bot removed the lgtm label Jul 19, 2023
@jotak
Copy link
Member Author

jotak commented Jul 19, 2023

thanks @nathan-weinberg @skrthomas - I updated with your suggestion .. and also found another outdated stuff regarding exporters, still mentioned as experimental

Co-authored-by: Sara Thomas <sarthoma@redhat.com>
@jotak jotak merged commit 0d6ebe8 into netobserv:main Jul 20, 2023
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants