Skip to content

generate cli reference docs#800

Merged
jetstack-bot merged 8 commits intomasterfrom
generate-cli-reference-docs
Jan 25, 2022
Merged

generate cli reference docs#800
jetstack-bot merged 8 commits intomasterfrom
generate-cli-reference-docs

Conversation

@jahrlin
Copy link
Contributor

@jahrlin jahrlin commented Jan 20, 2022

Fixes cert-manager/cert-manager#4458

adds a new function genclireference that will attempt to run main.go --help for our CLIs and writes the output to markdown files for each version of the docs.

contains some crude checks to cover some corner cases where some CLIs don't exist for older releases, or when they don't produce any output from --help (pre-cobra)

the preview deployment does not seem to include generated docs.

result examples:
image
image

Signed-off-by: Joakim Ahrlin <joakim.ahrlin@gmail.com>
Signed-off-by: Joakim Ahrlin <joakim.ahrlin@gmail.com>
Signed-off-by: Joakim Ahrlin <joakim.ahrlin@gmail.com>
Signed-off-by: Joakim Ahrlin <joakim.ahrlin@gmail.com>
@jetstack-bot jetstack-bot added dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 20, 2022
@netlify
Copy link

netlify bot commented Jan 20, 2022

✔️ Deploy Preview for cert-manager-website ready!

🔨 Explore the source changes: 52afa25

🔍 Inspect the deploy log: https://app.netlify.com/sites/cert-manager-website/deploys/61efee0d64c00500086bfbe7

😎 Browse the preview: https://deploy-preview-800--cert-manager-website.netlify.app

Signed-off-by: Joakim Ahrlin <joakim.ahrlin@gmail.com>
@jahrlin
Copy link
Contributor Author

jahrlin commented Jan 21, 2022

/retest

@jahrlin
Copy link
Contributor Author

jahrlin commented Jan 21, 2022

/retest

Signed-off-by: Joakim Ahrlin <joakim.ahrlin@gmail.com>
update gitignore

Signed-off-by: Joakim Ahrlin <joakim.ahrlin@gmail.com>
@jetstack-bot jetstack-bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 24, 2022
@jahrlin
Copy link
Contributor Author

jahrlin commented Jan 24, 2022

Updated after discussion with team today.

Previous version CLI reference docs are added to git, current and next version will always be generated - as they are likely to change while you are working with the docs


mkdir -p "${REPO_ROOT}/content/en/${outputdir}/cli/"

output=$(go run "$target/main.go" --help)
Copy link
Contributor

Choose a reason for hiding this comment

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

question: was continuing the work of generating the documentation via cobra considered? https://github.com/jetstack/cert-manager/blob/master/tools/cobra/main.go

Not a huge thing, but it does give a sightlier better output format, and can be configured with headers etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Going forward it could definitely make sense to build on top of that, the new website will not be based on Hugo so it could be we need something purpose-built rather than this simple script.

At this stage we decided to simply use the output of --help, and we wanted the CLI reference for older versions.

Copy link
Contributor Author

@jahrlin jahrlin Jan 24, 2022

Choose a reason for hiding this comment

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

Tested it a bit now and it's not a huge change to use the tool you linked for the versions where it's available, but it requires some work on both ends and I'm not sure it's worth it? Given the upcoming website refresh

Copy link
Member

Choose a reason for hiding this comment

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

Thought: I'm surprised that --help doesn't output to standard error.

Copy link
Member

@jakexks jakexks 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 definitely an improvement over not having anything at all, so I think it should merge.

I agree with Josh that the output format could use some work in future, but the website is about to go through a big style overhaul so I'm not too worried about it right now.

The .gitignore overrides were discussed with others in the team, I have no real opinion on this as long as it doesn't conflict with the other doc generation steps.

/lgtm
/hold

Placing a hold for anyone who chatted with @jahrlin earlier to also review, if they like.


mkdir -p "${REPO_ROOT}/content/en/${outputdir}/cli/"

output=$(go run "$target/main.go" --help)
Copy link
Member

Choose a reason for hiding this comment

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

Thought: I'm surprised that --help doesn't output to standard error.

@jetstack-bot jetstack-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 24, 2022
@jetstack-bot jetstack-bot added the lgtm Indicates that a PR is ready to be merged. label Jan 24, 2022
@jetstack-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jahrlin, jakexks

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@jetstack-bot jetstack-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 24, 2022
Signed-off-by: Joakim Ahrlin <joakim.ahrlin@gmail.com>
@jetstack-bot jetstack-bot removed the lgtm Indicates that a PR is ready to be merged. label Jan 25, 2022
@jakexks
Copy link
Member

jakexks commented Jan 25, 2022

/lgtm

@jetstack-bot jetstack-bot added the lgtm Indicates that a PR is ready to be merged. label Jan 25, 2022
@jahrlin
Copy link
Contributor Author

jahrlin commented Jan 25, 2022

/unhold

@jetstack-bot jetstack-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 25, 2022
@jetstack-bot jetstack-bot merged commit 6aa4474 into master Jan 25, 2022
@wallrj wallrj linked an issue Jan 27, 2022 that may be closed by this pull request
This was referenced Jan 27, 2022
@SgtCoDFish SgtCoDFish deleted the generate-cli-reference-docs branch April 26, 2022 10:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. lgtm Indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Automatically generate markdown help for all command line flags Document component flags

4 participants