Skip to content

Reinstate the helm command docs#401

Merged
flynnduism merged 6 commits intohelm:masterfrom
flynnduism:docs-helm-cmds
Jan 29, 2020
Merged

Reinstate the helm command docs#401
flynnduism merged 6 commits intohelm:masterfrom
flynnduism:docs-helm-cmds

Conversation

@flynnduism
Copy link
Copy Markdown
Member

@flynnduism flynnduism commented Nov 20, 2019

Needs help before review

Issue #383 flagged some content that we're missing since we moved rom v2 to v3 docs. Changes in where the docs are stored and how they're built has impacted on their inclusion, but IMO we should display them here if at all possible.

I've made a start on that here, but with content that is out of date. This PR contains contains content from v2. Before this PR is mergeable I need to run make docs (or whatever the new process is) to extract the latest markdown content from helm/helm.

Checklist:

  • export latest helper content for all helm _____ commands
  • import markdown
  • index with front matter as needed
  • ensure new section renders nav links
  • ensure templates can properly display browsable list commands

@StephenBrown2
Copy link
Copy Markdown
Contributor

StephenBrown2 commented Nov 25, 2019

Minor note: Looks like those docs were auto-generated with cobra: https://github.com/spf13/cobra/blob/master/doc/md_docs.md which even has an example for kubectl.

Update: helm has a hidden command helm docs that will do just that.

@bacongobbler
Copy link
Copy Markdown
Member

bacongobbler commented Nov 25, 2019

Originally, we removed this section from the documentation as it was a massive blocker for users wishing to contribute documentation to Helm.

In order to re-build the CLI reference, you required the ability to build the helm CLI from source, which for many users is a major blocker for contributing to the docs... There very few users who are comfortable with Go, and even fewer comfortable with Go and Hugo.

How do we reconcile those concerns?

The only way I can imagine we address that is by setting up a netlify script that pulls the latest version of Helm down and re-generates the CLI reference every build. Perhaps that's an option here?

@StephenBrown2
Copy link
Copy Markdown
Contributor

The only way I can imagine we address that is by setting up a netlify script that pulls the latest version of Helm down and re-generates the CLI reference every build. Perhaps that's an option here?

That would be a good option, I think, though there would need to be some minor massaging before it'd be good to go, namely: the header information (title and description) need to be grabbed from the first three lines of the generated docs, and the SEE ALSO links at the bottom need to be redirected to ../ and have the .md extension removed.

@bacongobbler
Copy link
Copy Markdown
Member

bacongobbler commented Nov 25, 2019

helm docs has an option to be rendered as HTML (via --output html). Hugo accepts both markdown as well as raw HTML. That could be used to work around that.

@StephenBrown2
Copy link
Copy Markdown
Contributor

html output would be good, though I didn't see that as an option when I ran helm docs --help:

Generate documentation files for Helm.

This command can generate documentation for Helm in the following formats:

- Markdown
- Man pages

It can also generate bash autocompletions.

Usage:
  helm docs [flags]

Flags:
      --dir string    directory to which documentation is written (default "./")
  -h, --help          help for docs
      --type string   the type of documentation to generate (markdown, man, bash) (default "markdown")

The links would still need to be adjusted.

@bacongobbler
Copy link
Copy Markdown
Member

I stand corrected. Either way, we were able to work around it in Helm 2's documentation; I'm sure we can take a similar approach here as well.

@mattfarina
Copy link
Copy Markdown
Contributor

@flynnduism where does this stand? I see one final task in the checkboxes.

@flynnduism
Copy link
Copy Markdown
Member Author

@mattfarina this PR reinstates that section to the site, updates menus etc. However the uncertain part is extracting the latest helm cmd help content from main project repo itself. The content here in this PR is still based on v2.

In v2, mkdocs exported markdown for this and we rendered this within the docs website. The script that published it is no longer in place. I need help figuring out how to export the most-up-to-date help text to publish it here.

@flynnduism
Copy link
Copy Markdown
Member Author

Actually I can do this with the info @bacongobbler provided - running helm docs --type markdown generates the content I need to complete this PR.

@flynnduism flynnduism force-pushed the docs-helm-cmds branch 2 times, most recently from 1f1f53c to f1fca54 Compare January 13, 2020 21:18
@flynnduism
Copy link
Copy Markdown
Member Author

Screen Shot 2020-01-13 at 1 19 44 PM

Ok the latest help text content has been imported and indexed. This is ready for review:
https://5e1cdeae86b7440007fd1286--helm-merge.netlify.com/docs/helm/helm/

@flynnduism
Copy link
Copy Markdown
Member Author

I noted that two of the CLI commands from v2 are not documented here.

helm get notes
helm inspect readme

Assuming these are depreciated?

@bacongobbler
Copy link
Copy Markdown
Member

bacongobbler commented Jan 14, 2020

they weren't ported over I think. I believe they're open issues for Helm 3 to bring back

@bacongobbler
Copy link
Copy Markdown
Member

We can add them to the CLI reference once they've been reintroduced

@bacongobbler
Copy link
Copy Markdown
Member

Hey @flynnduism! Is this PR ready for another review? Let me know and I can take another look. Thanks!

@flynnduism
Copy link
Copy Markdown
Member Author

flynnduism commented Jan 17, 2020

@bacongobbler yep this is good to review.

I was unsure about the missing content - we can follow up separately with the docs for helm get notes andhelm inspect readme once those are reinstated in the main project.

Copy link
Copy Markdown
Member

@bacongobbler bacongobbler left a comment

Choose a reason for hiding this comment

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

Would you mind adding a note in the README on how to generate this documentation for future reference? That way community members can re-generate the documentation as needed when they introduce new features to Helm.

Content looks good to me!

@flynnduism
Copy link
Copy Markdown
Member Author

@bacongobbler thanks, I've updated the readme

Copy link
Copy Markdown
Member

@bacongobbler bacongobbler left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks Ronan. Merge when ready.

@StephenBrown2
Copy link
Copy Markdown
Contributor

The links from the see also pages don't line up with the paths on the site. e.g.
helm home on the index goes to /docs/helm/helm/helm_home.md rather than /docs/helm/helm_home/ where it exists.

@flynnduism
Copy link
Copy Markdown
Member Author

Good catch @StephenBrown2. I added a fix to correct those see also links.
Preview here. If that looks 👍 I will merge this.

@StephenBrown2
Copy link
Copy Markdown
Contributor

LGTM, should there also be a step in the README to fix them on update?

@flynnduism
Copy link
Copy Markdown
Member Author

LGTM, should there also be a step in the README to fix them on update?

Yep, this PR updates the Readme to do so here.

@flynnduism flynnduism merged commit 60d724a into helm:master Jan 29, 2020
@StephenBrown2
Copy link
Copy Markdown
Contributor

Yes, that's what I linked[1], but there isn't a step to fix the SEE ALSO links.

[1] the README: https://github.com/helm/helm-www/pull/401/files#diff-04c6e90faac2675aa89e2176d2eec7d8R46-R53

@flynnduism
Copy link
Copy Markdown
Member Author

Yes, that's what I linked[1], but there isn't a step to fix the SEE ALSO links.

D'oh, I'm sorry. That makes total sense @StephenBrown2. I will amend with a follow up PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants