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.

Sign up for GitHub

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

Jump to bottom

[bitnami/*] Standardize documentation #19835

Merged
merged 1 commit into from
Oct 20, 2023
Merged

[bitnami/*] Standardize documentation #19835

merged 1 commit into from
Oct 20, 2023

Conversation

jotamartos
Copy link
Contributor

Description of the change

Standardize documentation for all charts in the catalog

Benefits

Documentation can be reused in all projects. It's not dependent of the distro or repository information.

Possible drawbacks

Applicable issues

Additional information

Checklist

  • [NA] Chart version bumped in Chart.yaml according to semver. This is not necessary when the changes only affect README.md files.
  • [NA] Variables are documented in the values.yaml and added to the README.md using readme-generator-for-helm
  • Title of the pull request follows this pattern [bitnami/<name_of_the_chart>] Descriptive title
  • All commits signed off and in agreement of Developer Certificate of Origin (DCO)

@bitnami-bot bitnami-bot added the verify Execute verification workflow for these changes label Oct 9, 2023
@jotamartos jotamartos removed the request for review from CeliaGMqrz October 9, 2023 09:28
@jotamartos jotamartos mentioned this pull request Oct 9, 2023
Merged
2 tasks
carrodher
carrodher reviewed Oct 16, 2023
View reviewed changes
Copy link
Member

@carrodher carrodher left a comment

Choose a reason for hiding this comment

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

Thanks for the changes, in general, they LGTM. Just a comment that could be applied to the rest of sections

bitnami/discourse/README.md Outdated Show resolved Hide resolved
bitnami/discourse/README.md Outdated Show resolved Hide resolved
@raquel-campuzano
Copy link
Contributor

Please apply the comments related to placeholders for consistency with the rest of documentation. Thank you!

@github-actions github-actions bot added the triage Triage is needed label Oct 17, 2023
@github-actions github-actions bot removed the triage Triage is needed label Oct 17, 2023
@jotamartos
Copy link
Contributor Author

Waiting for bitnami/readme-generator-for-helm#78 before updating all README files in this PR.

@jotamartos jotamartos force-pushed the standardize-documentation branch 3 times, most recently from 21f4fe9 to e32eb0a Compare October 19, 2023 13:43
@jotamartos jotamartos force-pushed the standardize-documentation branch 4 times, most recently from 78f1825 to f46027f Compare October 19, 2023 14:24
carrodher
carrodher reviewed Oct 19, 2023
View reviewed changes
README.md Outdated
```

> Note: You need to substitute the placeholders REGISTRY_NAME and REPOSITORY_NAME with a reference to your Helm chart registry and repository. For example, in the case of Bitnami, you need to use REGISTRY_NAME=registry-1.docker.io and REPOSITORY_NAME=bitnamicharts.
Copy link
Member

Choose a reason for hiding this comment

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

Note: You need to substitute the placeholders REGISTRY_NAME and REPOSITORY_NAME with a reference to your Helm chart registry and repository. For example, in the case of Bitnami, you need to use REGISTRY_NAME=registry-1.docker.io and REPOSITORY_NAME=bitnamicharts.

Changes LGTM overall, just I would add the placeholders as a text field (see above example) or at least in italics, but differentiate from regular text in some way. WDYT @raquel-campuzano?

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 info looks better this way. I just applied the changes.

@jotamartos jotamartos force-pushed the standardize-documentation branch 3 times, most recently from 0e9e325 to 7d733db Compare October 19, 2023 16:46
carrodher
carrodher previously approved these changes Oct 19, 2023
View reviewed changes
fmulero
fmulero previously approved these changes Oct 20, 2023
View reviewed changes
Copy link
Collaborator

@fmulero fmulero left a comment

Choose a reason for hiding this comment

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

LGTM

@jotamartos jotamartos dismissed stale reviews from fmulero and carrodher via 0bb71e5 October 20, 2023 06:15
@jotamartos jotamartos enabled auto-merge (squash) October 20, 2023 06:17
@jotamartos jotamartos merged commit af5f753 into main Oct 20, 2023
11 of 12 checks passed
@jotamartos jotamartos deleted the standardize-documentation branch October 20, 2023 06:18
This was referenced Oct 20, 2023
Merged
Merged
@FraPazGal FraPazGal mentioned this pull request Nov 7, 2023
Merged
2 tasks
garettmd pushed a commit to garettmd/odoo that referenced this pull request Jan 31, 2024
@jotamartos
[bitnami/*] Standardize documentation (bitnami#19835)
ca5ce90 
Signed-off-by: Jota Martos <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fmulero fmulero fmulero approved these changes

@raquel-campuzano raquel-campuzano Awaiting requested review from raquel-campuzano

@carrodher carrodher Awaiting requested review from carrodher

Assignees

@jotamartos jotamartos

@carrodher carrodher

Labels
bitnami discourse verify Execute verification workflow for these changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@jotamartos @raquel-campuzano @carrodher @fmulero @bitnami-bot @CeliaGMqrz