OPECO-3073: Add Note for CRD Version Spec #319
Conversation
| @@ -96,6 +96,7 @@ spec: | |||
| # name is the metadata.name of the CRD (which is of the form <plural>.<group>) | |||
| - name: memcacheds.cache.example.com | |||
| # version is the spec.versions[].name value defined in the CRD | |||
| # while version is not required, semver template functionality relies on its presence | |||
There was a problem hiding this comment.
I took a look at #311 and it looks like it is referring to the need for the spec.version field (https://github.com/operator-framework/olm-docs/pull/319/files#diff-0fd678129cce2ee24610d34be80b5bd6aaef3f7cc1191123eafb86e732b76dbcR45) to be present for the semver template. This PR, in it's current form, is stating that the CRD version is required and doesn't resolve #311 .
Additionally it might be helpful to add a note to https://olm.operatorframework.io/docs/reference/catalog-templates/#semver-template to mention the requirement for the CSV's spec.version field to be populated where folks may go back to reference information on the semver template or first learn about it.
There was a problem hiding this comment.
WDYT of the following?
| # while version is not required, semver template functionality relies on its presence | |
| # spec.version is an optional field. | |
| # the spec.version field is required if you want to use the semver template for your catalog. |
There was a problem hiding this comment.
I'm a little reluctant to update CSV docs at all because:
- if a user has no intention to order by spec.version, this is just noise.
- in this location it feels common-sense. "What do you mean that to sequence I have to provide a value?" feels like a question that has an obvious answer.
- the reliance on spec.version actually lies in semver catalog template, which is alpha software, so it feels awkward allowing alpha software to impose any burden on core implementation w/o the usual gradual adoption workflow.
I think it makes more sense to put the caveat in semver template area in line with #311's note. Ideally that would also include a reference to the CSV location to show the field in question.
|
Please update the PR description to include |
|
Updated the semver composite template page - will remove the line from the other file once we agree on the wording here, just wanted to keep that for now as there are discussion comments tied to it. |
Signed-off-by: Ish Shah <[email protected]>
Signed-off-by: Ish Shah <[email protected]>
Co-authored-by: Michael Peter <[email protected]> Signed-off-by: Ish Shah <[email protected]>
theishshah
force-pushed
the
theishshah-patch-1
branch
from
e953a30 to
54f1dc3
Compare
|
@grokspawn DCO is cleared up, and text is reworded in the right spot should be good for another review. |
grokspawn
changed the title
Signed-off-by: Ish Shah <[email protected]>
theishshah
force-pushed
the
theishshah-patch-1
branch
from
a4a236a to
4ff8f7b
Compare
Signed-off-by: Ish Shah <[email protected]>
|
/jira refresh |
This addresses the note from #311 informing users that the version spec is necessary for semver templates.
Solves #311