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

Create documentation for bundles docs #3804

Merged
merged 5 commits into from
Apr 29, 2019
Merged

Create documentation for bundles docs #3804

merged 5 commits into from
Apr 29, 2019

Conversation

polskikiel
Copy link
Contributor

Description

Changes proposed in this pull request:

  • Created documentation for bundles docs

Related issue(s)

```
For now, we support only one entry in the `docs` array.

The Helm Broker can provision a broker which provides its own ServiceClasses. You can see how to provide a documentation for them in the following [document](#details-bundles-docs).
Copy link
Contributor

Choose a reason for hiding this comment

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

is this link working?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This link will work on kyma-project.io, we must define it in this way because of governance job which blocks links to the not existing documents.

url: {.Values.addonsRepositoryURL}
filter: docs/{class_name}/ # provide filter only if you use the package mode
___
# it is prefered to store all of the ClusterDocsTopics definitions in one file
Copy link
Contributor

Choose a reason for hiding this comment

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

this sentence: "It is preferred" should be moved from the example to the normal documentation paragraph

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

If the `docs/meta.yaml` is specified the Helm Broker tries to create the ClusterDocsTopic for this bundle. Below you can see the example structure of the `meta.yaml` file.
```
docs:
- template: # template describes the ClusterDocsTopic that will be created.
Copy link
Contributor

Choose a reason for hiding this comment

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

we provided an example, but where I can learn more information about which fields I can provide?

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 added a sentence about this below the example

cms.kyma-project.io/view-context: service-catalog
name: {ServiceClass ID}
spec:
displayName: "Slack Connector Add-on"
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we use placeholder rather than examples?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@klaudiagrz What do you think?

Copy link

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Yup, I'd go with placeholders as well. @polskikiel please apply this suggestion.

docs/helm-broker/docs/03-01-create-bundles.md Outdated Show resolved Hide resolved
docs/helm-broker/docs/03-01-create-bundles.md Outdated Show resolved Hide resolved
docs/helm-broker/docs/03-01-create-bundles.md Outdated Show resolved Hide resolved
docs/helm-broker/docs/03-01-create-bundles.md Outdated Show resolved Hide resolved
docs/helm-broker/docs/03-01-create-bundles.md Outdated Show resolved Hide resolved
sources: # list of files to upload as a Asset
- type: markdown
name: markdown-files # must be unique within sources
mode: package # defines that file under below
Copy link
Contributor

Choose a reason for hiding this comment

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

What?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed xD

docs/helm-broker/docs/03-01-create-bundles.md Outdated Show resolved Hide resolved
url: abc.pl # if not provided then Helm broker will inject the full repository URL for this bundle
filter: /docs/bundles
```
For more information about the fields you can provide and ClusterDocsTopics see the following [file](https://kyma-project.io/docs/components/headless-cms/#custom-resource-clusterdocstopic).
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this sentence and provide the link to the CLuterDocsTopics in a place when you mention it for the first time.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

docs/helm-broker/docs/03-01-create-bundles.md Outdated Show resolved Hide resolved
docs/helm-broker/docs/03-01-create-bundles.md Outdated Show resolved Hide resolved
Copy link

@kazydek kazydek left a comment

Choose a reason for hiding this comment

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

I added some comments but I would prefer that the topic owner @klaudiagrz had a final look at it :)

docs/helm-broker/docs/03-04-bundles-docs.md Outdated Show resolved Hide resolved
docs/helm-broker/docs/03-04-bundles-docs.md Outdated Show resolved Hide resolved
docs/helm-broker/docs/03-04-bundles-docs.md Outdated Show resolved Hide resolved
docs/helm-broker/docs/03-04-bundles-docs.md Outdated Show resolved Hide resolved
docs/helm-broker/docs/03-04-bundles-docs.md Outdated Show resolved Hide resolved
docs/helm-broker/docs/03-04-bundles-docs.md Outdated Show resolved Hide resolved
docs/helm-broker/docs/03-04-bundles-docs.md Outdated Show resolved Hide resolved
docs/helm-broker/docs/03-04-bundles-docs.md Outdated Show resolved Hide resolved
docs/helm-broker/docs/03-04-bundles-docs.md Outdated Show resolved Hide resolved
cms.kyma-project.io/view-context: service-catalog
name: {ServiceClass ID}
spec:
displayName: "Slack Connector Add-on"
Copy link

Choose a reason for hiding this comment

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

Copy link

@derberg derberg left a comment

Choose a reason for hiding this comment

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

  • this PR is missing information about markdown files, how they should look like, that they should have metadata (metadata is not native for markdown) and what metadata actually and how they will behave
  • you are kind of missing a nice tutorial, tbh best dock would be if you would have a simple tutorial about bundle creation that would be based on https://github.com/kyma-project/bundles/tree/master/bundles/testing-0.0.1 and this bundle actually should be extended with documentation:
    • you could use it as reverence in the tutorial
    • so later it can also be used for testing of the catalog and instance views ;)

polskikiel and others added 2 commits April 25, 2019 10:37
Co-Authored-By: polskikiel <PolskiKiel@gmail.com>
@aszecowka
Copy link
Contributor

/test pre-master-kyma-guard-job

@adamwalach
Copy link
Contributor

/test kyma-service-catalog-e2e-tests

4 similar comments
@adamwalach
Copy link
Contributor

/test kyma-service-catalog-e2e-tests

@adamwalach
Copy link
Contributor

/test kyma-service-catalog-e2e-tests

@adamwalach
Copy link
Contributor

/test kyma-service-catalog-e2e-tests

@adamwalach
Copy link
Contributor

/test kyma-service-catalog-e2e-tests

@polskikiel
Copy link
Contributor Author

@polskikiel
Copy link
Contributor Author

/retest

1 similar comment
@polskikiel
Copy link
Contributor Author

/retest

Copy link
Contributor

@klaudiagrz klaudiagrz left a comment

Choose a reason for hiding this comment

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

This is not the end of the review. Let's discuss the structure of the document.

docs/helm-broker/docs/03-01-create-bundles.md Outdated Show resolved Hide resolved
docs/helm-broker/docs/03-01-create-bundles.md Outdated Show resolved Hide resolved
## docs directory

In the `docs` directory, provide documentation for your bundle and for the wrapped Helm chart. You can also define a `meta.yaml` file inside the `docs` folder, which provides information on how documentation for the bundle is uploaded.
As the Helm Broker is installed as a ClusterServiceBroker, documentation for bundles is provided using ClusterDocsTopics.
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a link to the ClusterDocsTopic.


In the `docs` directory, provide documentation for your bundle and for the wrapped Helm chart. You can also define a `meta.yaml` file inside the `docs` folder, which provides information on how documentation for the bundle is uploaded.
As the Helm Broker is installed as a ClusterServiceBroker, documentation for bundles is provided using ClusterDocsTopics.
If you specify the `meta.yaml`, the Helm Broker creates a ClusterDocsTopic for this bundle based on the `meta.yaml` file. The example structure of the `meta.yaml` file looks as follows:
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd remove this whole sentence - meta.yaml is obligatory.


>**NOTE:** Currently you can provide only one entry in the `docs` array. In the future we will enable many entries in case of many charts in one bundle.

The `meta.yaml` file contains the specification of the ClusterDocsTopic. To learn more about it, read [this](/components/headless-cms/#custom-resource-clusterdocstopic) document.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd move this sentence above the table.

docs/helm-broker/docs/03-01-create-bundles.md Outdated Show resolved Hide resolved
docs/helm-broker/docs/03-01-create-bundles.md Outdated Show resolved Hide resolved
cms.kyma-project.io/view-context: service-catalog
name: {ServiceClass ID}
spec:
displayName: "Slack Connector Add-on"
Copy link
Contributor

Choose a reason for hiding this comment

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

Yup, I'd go with placeholders as well. @polskikiel please apply this suggestion.

docs/helm-broker/docs/03-04-bundles-docs.md Outdated Show resolved Hide resolved
docs/helm-broker/docs/03-04-bundles-docs.md Show resolved Hide resolved
Store documentation for each Service Class in the `docs/{class_name}` directory which corresponds to the value of the **filter** parameter in the ClusterDocsTopic definition.


During the provisioning process, the Helm Broker pushes the **addonRepositoryURL** variable into the chart. The **addonsRepositoryURL** points to your bundle compressed to a `.tgz` file.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure about this paragraph. To be honest, I would totally omit this info. @PK85 let us know what you think :)

@polskikiel
Copy link
Contributor Author

/retest

@kyma-bot
Copy link
Contributor

kyma-bot commented Apr 26, 2019

@polskikiel: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
kyma-service-catalog-e2e-tests 4c5793c link /test kyma-service-catalog-e2e-tests

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.

@polskikiel polskikiel merged commit 88955de into kyma-project:master Apr 29, 2019
@polskikiel polskikiel deleted the bundles-docs-docs branch April 29, 2019 08:13
@adamwalach
Copy link
Contributor

/test kyma-service-catalog-e2e-tests

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

7 participants