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

CRD documentation list all versions #17421

Merged
merged 22 commits into from
May 9, 2023

Conversation

k15r
Copy link
Contributor

@k15r k15r commented Apr 27, 2023

Description

Changes proposed in this pull request:

  • create one section per crd version
  • create one table per spec and status
  • add "type" column with indicator if mandatory

Related issue(s)

@k15r k15r requested a review from a team as a code owner April 27, 2023 16:12
@netlify
Copy link

netlify bot commented Apr 27, 2023

Deploy Preview for kyma-project-docs-preview ready!

Name Link
🔨 Latest commit f09c35b
🔍 Latest deploy log https://app.netlify.com/sites/kyma-project-docs-preview/deploys/645a23b153f9b0000841b0f6
😎 Deploy Preview https://deploy-preview-17421--kyma-project-docs-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@kyma-bot kyma-bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Apr 27, 2023
@k15r k15r requested a review from a team as a code owner April 27, 2023 16:15
@kyma-bot kyma-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Apr 27, 2023
<!-- TracePipeline v1alpha1 telemetry.kyma-project.io -->
| Parameter | Type | Description |
| ------------------| ---- | --------------------------------------------- |
| **spec.output** | object | Defines a destination for shipping trace data. Only one can be defined per pipeline. |
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you remove the "spec." prefix, it is redundant and will reduce the width of the column

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure

@a-thaler
Copy link
Contributor

Could you ad the "required" flag behind the type so that we have covered all requirements already. That would be great :)

@k15r
Copy link
Contributor Author

k15r commented Apr 28, 2023

Could you ad the "required" flag behind the type so that we have covered all requirements already. That would be great :)

let me try. but as this tool uses a rather basic way to "parse" openapi it will not be nice

@k15r k15r closed this Apr 28, 2023
@k15r k15r reopened this Apr 28, 2023
<!-- Subscription v1alpha1 eventing.kyma-project.io -->
| Parameter | Type | Description |
| ------------------| ---- | --------------------------------------------- |
| **** | object | SubscriptionSpec defines the desired state of Subscription. |
Copy link
Contributor

Choose a reason for hiding this comment

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

The "spec" root attribute need to be excluded from generation

| ------------------| ---- | --------------------------------------------- |
| **** | object | Defines the desired state of LogPipeline |
| **files** | Provides file content to be consumed by a LogPipeline configuration | files |
Copy link
Contributor

Choose a reason for hiding this comment

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

The description is in second column...

@kyma-bot kyma-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 3, 2023
@k15r k15r linked an issue May 5, 2023 that may be closed by this pull request
4 tasks
Copy link
Contributor

@mmitoraj mmitoraj left a comment

Choose a reason for hiding this comment

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

Hi, nice new features :) I have a few suggestions:

  • Please remove all parameters' names from the Description to avoid duplication. For example, instead of:
| **backend.emshash** | integer | Emshash defines the hash for the Subscription in EventType. |

write:

| **backend.emshash** | integer | Defines the hash for the Subscription in EventType. |
  • For me, the information if something is required doesn't look good in the Type column. How about adding it in parentheses in a new line after the parameter name in the Parameter column? For example:
 | **protocolsettings.webhookAuth.tokenUrl** (required) | string | TokenURL defines token URL for OAuth2 |

It should go to a new line under the parameter name, right?

  • I added a few suggestions to the descriptions themselves but is there a point of doing that in the autogenerated document? Can I review those in the source files?


| Parameter | Type | Description |
| ---- | ----------- | ---- |
| **config** | map[string]string | Config defines the configurations that can be applied to the eventing backend. |
Copy link
Contributor

Choose a reason for hiding this comment

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

"map[string]string" in the preview displays as "mapstringstring". I guess it should look differently, doesn't it?

| ---- | ----------- | ---- |
| **config** | map[string]string | Config defines the configurations that can be applied to the eventing backend. |
| **id** | string | ID is the unique identifier of Subscription, read-only. |
| **sink** | string **required** | Sink defines endpoint of the subscriber |
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
| **sink** | string **required** | Sink defines endpoint of the subscriber |
| **sink** | string **required** | Defines the subscriber's endpoint. |

| **sink** | string **required** | Sink defines endpoint of the subscriber |
| **source** | string **required** | Source Defines the source of the event originated from. |
| **typeMatching** | string | TypeMatching defines the type of matching to be done for the event types. |
| **types** | []string | Types defines the list of event names for the topics we need to subscribe for messages. |
Copy link
Contributor

Choose a reason for hiding this comment

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

Is "[ ] string" a string array?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes it is

| Parameter | Type | Description |
| ---- | ----------- | ---- |
| **config** | map[string]string | Config defines the configurations that can be applied to the eventing backend. |
| **id** | string | ID is the unique identifier of Subscription, read-only. |
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
| **id** | string | ID is the unique identifier of Subscription, read-only. |
| **id** | string | The unique identifier of the Subscription, read-only. |

| **config** | map[string]string | Config defines the configurations that can be applied to the eventing backend. |
| **id** | string | ID is the unique identifier of Subscription, read-only. |
| **sink** | string **required** | Sink defines endpoint of the subscriber |
| **source** | string **required** | Source Defines the source of the event originated from. |
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
| **source** | string **required** | Source Defines the source of the event originated from. |
| **source** | string **required** | The origin from which events are published.|

| **id** | string | ID is the unique identifier of Subscription, read-only. |
| **sink** | string **required** | Sink defines endpoint of the subscriber |
| **source** | string **required** | Source Defines the source of the event originated from. |
| **typeMatching** | string | TypeMatching defines the type of matching to be done for the event types. |
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
| **typeMatching** | string | TypeMatching defines the type of matching to be done for the event types. |
| **typeMatching** | string | Defines the matching type for event types. |

| **sink** | string **required** | Sink defines endpoint of the subscriber |
| **source** | string **required** | Source Defines the source of the event originated from. |
| **typeMatching** | string | TypeMatching defines the type of matching to be done for the event types. |
| **types** | []string | Types defines the list of event names for the topics we need to subscribe for messages. |
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
| **types** | []string | Types defines the list of event names for the topics we need to subscribe for messages. |
| **types** | []string | Defines the list of event names for the topics we need to subscribe to for messages. |

@mmitoraj mmitoraj mentioned this pull request May 8, 2023
5 tasks
@k15r k15r requested a review from Teneroy May 8, 2023 13:02
Copy link
Contributor

@mmitoraj mmitoraj left a comment

Choose a reason for hiding this comment

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

In the Description column links don’t display correctly and the formatting is broken (code font, bold, etc.).


| Parameter | Type | Description |
| ---- | ----------- | ---- |
| **parser** | string | \[Fluent Bit Parsers\]\(https://docs\.fluentbit\.io/manual/pipeline/parsers\)\. The parser specified here has no effect until it is referenced by a \[Pod annotation\]\(https://docs\.fluentbit\.io/manual/pipeline/filters/kubernetes\#kubernetes\-annotations\) on your workload or by a \[Parser Filter\]\(https://docs\.fluentbit\.io/manual/pipeline/filters/parser\) defined in a pipeline's filters section\. |
Copy link
Contributor

Choose a reason for hiding this comment

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

Links don't display correctly throughout all the documents.

| **conditions** | array | An array of conditions describing the status of the parser\. |
| **conditions.lastTransitionTime** | string | An array of conditions describing the status of the parser\. |
| **conditions.reason** | string | An array of conditions describing the status of the parser\. |
| **conditions.type** | string | The possible transition types are:\<br\>\- \`Running\`: The parser is ready and usable\.\<br\>\- \`Pending\`: The parser is being activated\. |
Copy link
Contributor

Choose a reason for hiding this comment

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

Formatting in the Description column doesn't display correctly throughout all the documents.

Teneroy
Teneroy previously approved these changes May 8, 2023
@kyma-bot kyma-bot added the lgtm Looks good to me! label May 8, 2023
@kyma-bot kyma-bot removed the lgtm Looks good to me! label May 9, 2023
@kyma-bot kyma-bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels May 9, 2023
they can then be updated with a followup pr
@kyma-bot kyma-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels May 9, 2023
@kyma-bot
Copy link
Contributor

kyma-bot commented May 9, 2023

@k15r: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pre-main-kyma-gardener-gcp-eventing-upgrade 2089d9f link true /test pre-main-kyma-gardener-gcp-eventing-upgrade

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@k15r k15r requested a review from Teneroy May 9, 2023 10:45
@kyma-bot kyma-bot added the lgtm Looks good to me! label May 9, 2023
@kyma-bot kyma-bot merged commit 83054f4 into kyma-project:main May 9, 2023
12 checks passed
@k15r k15r added the area/documentation Issues or PRs related to documentation label May 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/documentation Issues or PRs related to documentation lgtm Looks good to me! size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow automatic generation CRD documentation
6 participants