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 CSV description to comply with guidelines #374

Merged
merged 3 commits into from
Apr 17, 2019
Merged

Update CSV description to comply with guidelines #374

merged 3 commits into from
Apr 17, 2019

Conversation

objectiser
Copy link
Contributor

Signed-off-by: Gary Brown gary@brownuk.com

Signed-off-by: Gary Brown <gary@brownuk.com>
@objectiser
Copy link
Contributor Author

@JStickler would you be able to review?

Copy link

@JStickler JStickler left a comment

Choose a reason for hiding this comment

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

@objectiser just a couple of comments and questions. Other than this, looks good.


* **Multiple modes** - Supports `allInOne`, `production`, and `streaming` [modes of deployment](https://github.com/jaegertracing/jaeger-operator#strategies).

* **Configuration** - Directly pass down all supported Jaeger configuration through the Operator.

* **Storage** - Configure storage used by Jaeger. By default, `memory` is used.
* **Storage** - Configure storage used by Jaeger. By default, `memory` is used. Other options include `elasticsearch` or `cassandra`. On openshift, the operator can delegate creation of an Elasticsearch cluster to the Elasticsearch Operator if deployed.

Choose a reason for hiding this comment

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

Since this is the community template, should the reference to OpenShift be to OKD instead since that is the upstream community? (If not, then camelcase OpenShift please.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Want to minimise the differences between the CSVs, so will use OpenShift as a general term.

* **Storage** - Configure storage used by Jaeger. By default, `memory` is used.
* **Storage** - Configure storage used by Jaeger. By default, `memory` is used. Other options include `elasticsearch` or `cassandra`. On openshift, the operator can delegate creation of an Elasticsearch cluster to the Elasticsearch Operator if deployed.

* **Agent** - can be deployed as sidecar and/or daemonset

Choose a reason for hiding this comment

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

Missing a period at the end.


### Before you start

1. Ensure that the appropriate storage solution, that will be used by the Jaeger instance, is available and configured

Choose a reason for hiding this comment

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

Missing period.

### Before you start

1. Ensure that the appropriate storage solution, that will be used by the Jaeger instance, is available and configured
1. If intending to deploy an Elasticsearch cluster via the Jaeger custom resource, then the Elasticsearch Operator must first be installed.

Choose a reason for hiding this comment

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

Should this be numbered as 2? Also "If you intend to deploy...."

Signed-off-by: Gary Brown <gary@brownuk.com>
@codecov
Copy link

codecov bot commented Apr 17, 2019

Codecov Report

Merging #374 into master will decrease coverage by 0.3%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #374      +/-   ##
==========================================
- Coverage   90.01%   89.71%   -0.31%     
==========================================
  Files          64       64              
  Lines        3076     3093      +17     
==========================================
+ Hits         2769     2775       +6     
- Misses        207      216       +9     
- Partials      100      102       +2
Impacted Files Coverage Δ
pkg/controller/jaeger/elasticsearch.go 50.72% <0%> (-5.05%) ⬇️

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 20c68a9...5af2e0c. Read the comment docs.

Signed-off-by: Gary Brown <gary@brownuk.com>
@objectiser
Copy link
Contributor Author

@JStickler Thanks.

Merging as codecov issue is unrelated to this PR.

@objectiser objectiser merged commit e190d03 into jaegertracing:master Apr 17, 2019
@objectiser objectiser deleted the csvdescription branch April 17, 2019 09:16
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.

2 participants