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

Documentation improvements #93

Merged
merged 5 commits into from
Sep 30, 2020
Merged

Conversation

seaneagan
Copy link
Contributor

@seaneagan seaneagan commented Sep 25, 2020

This attempts to work towards the extend documentation around managing Helm releases task mentioned in fluxcd/flux2#238:

The thought here is to have a leveled documentation, increasing detail in each level, and avoiding too much duplication between levels:

  1. https://toolkit.fluxcd.io/guides/helmreleases
  2. https://toolkit.fluxcd.io/components/helm/helmreleases/
  3. https://toolkit.fluxcd.io/components/helm/api/

This:

  • copies some of the content about HelmRelease CRD options from 1. to 2. with the idea being to remove or reduce it from 1. and replace with a more prominent link to 2. (as a follow up in the toolkit repo).
  • adds more documentation to 2.
  • removes some of the copy/pasted golang documentation from 2. since it already exists in 3. The status condition types / reasons are kept (though moved under the status section) since those are not currently included in 3. This could be potentially be resolved in the future by Emit documentation for typed constants ahmetb/gen-crd-api-reference-docs#21

Copy link
Member

@hiddeco hiddeco 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 putting time into this 🥝

I have to think for a bit about removing the structures from the spec docs, but do have some comments in the meantime.

docs/spec/README.md Outdated Show resolved Hide resolved
docs/spec/v2alpha1/helmreleases.md Outdated Show resolved Hide resolved
docs/spec/v2alpha1/helmreleases.md Outdated Show resolved Hide resolved
docs/spec/v2alpha1/helmreleases.md Outdated Show resolved Hide resolved
docs/spec/v2alpha1/helmreleases.md Outdated Show resolved Hide resolved
docs/spec/v2alpha1/helmreleases.md Outdated Show resolved Hide resolved
docs/spec/README.md Outdated Show resolved Hide resolved
@seaneagan seaneagan marked this pull request as ready for review September 29, 2020 15:37
@seaneagan seaneagan force-pushed the helmrelease-doc-updates branch 2 times, most recently from 29e39a6 to cf21f30 Compare September 29, 2020 16:21
- Add note about `dependsOn` and update ordering
- Replace references to "operator" with "controller" for consistency
- Add values override docs
This documents what is considered a desired state transition, and
the resulting upgrade and status condition semantics.
@hiddeco
Copy link
Member

hiddeco commented Sep 30, 2020

I have to think for a bit about removing the structures from the spec docs, but do have some comments in the meantime.

I discussed this a bit with @stefanprodan, and we both think the in-documentation code blocks should stay. Reason for this is that once we get out of the "rapid development cycle", new proposals should be made against the spec, and this becomes hard without them being available. We are currently working on more governance (including documentation) for the Flux project, and I will make sure this process will be documented somewhere (cc: @scottrigby).

I also think this means that the documentation within the spec that is added in this PR should be rewritten to something that describes desired application behaviour, instead of being explanatory to a user. But this can be taken care of as a follow up action, including moving bits and pieces to the HelmRelease guide (which targets users).

I took the liberty to rebase the PR and undo the code removals from the spec so this can be merged in the meantime, as we are trying to make new controller releases today and would like to see this included.

Copy link
Member

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

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

LGTM

GG @seaneagan and @hiddeco

@hiddeco hiddeco added the area/docs Documentation related issues and pull requests label Sep 30, 2020
@hiddeco hiddeco merged commit 4ddd98a into fluxcd:master Sep 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/docs Documentation related issues and pull requests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants