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

add "tink docs" command for markdown and man page generation #272

Merged
merged 2 commits into from
Sep 3, 2020
Merged

add "tink docs" command for markdown and man page generation #272

merged 2 commits into from
Sep 3, 2020

Conversation

displague
Copy link
Member

@displague displague commented Aug 28, 2020 •
edited
Loading

Description

This adds a tink docs command for generating both markdown and man pages.

This could be introduced in the Makefile as a way to ship and keep updated markdown docs in the repository, but I have not enabled that.

Why is this needed

Documentation about the tink commands is currently hand generated. Cobra allows for these to be autogenerated.

Man pages will help system operators learn about the available options. Man pages are also a requirement for Debian packages, should we want to explore that.

How Has This Been Tested?

make
mkdir docs
cmd/tink-cli/tink-cli docs markdown --path docs/

mkdir man
cmd/tink-cli/tink-cli docs man --path man/
man man/tink.1

How are existing users impacted? What migration steps/scripts do we need?

none

Checklist:

I have:

  • updated the documentation and/or roadmap (if required)
  • added unit or e2e tests
  • provided instructions on how to upgrade

displague
displague commented Aug 28, 2020
View reviewed changes
cmd/tink-cli/cmd/completion.go
@@ -32,10 +32,9 @@ func completionCmd(name string) *cobra.Command {
# To load completions for each session, execute once:
$ tink-cli completion fish > ~/.config/fish/completions/tink-cli.fish
`,
DisableFlagsInUseLine: true,
Copy link
Member Author

Choose a reason for hiding this comment

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

Removing this flag so that the completion command is included in --help.

@codecov
Copy link

codecov bot commented Aug 28, 2020 •
edited
Loading

Codecov Report

Merging #272 into master will increase coverage by 3.52%.
The diff coverage is 96.29%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #272      +/-   ##
==========================================
+ Coverage   17.08%   20.61%   +3.52%     
==========================================
  Files           9       15       +6     
  Lines        1264     1363      +99     
==========================================
+ Hits          216      281      +65     
- Misses       1036     1068      +32     
- Partials       12       14       +2
Impacted Files Coverage Δ
cmd/tink-cli/cmd/root.go 41.66% <ø> (ø)
cmd/tink-cli/cmd/docs.go 95.83% <95.83%> (ø)
cmd/tink-cli/cmd/completion.go 66.66% <100.00%> (ø)
cmd/tink-cli/cmd/hardware.go 42.85% <0.00%> (ø)
cmd/tink-cli/cmd/workflow.go 42.85% <0.00%> (ø)
cmd/tink-cli/cmd/template.go 42.85% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ac175e0...f0596c4. Read the comment docs.

displague
displague commented Aug 28, 2020
View reviewed changes
cmd/tink-cli/cmd/root.go
@@ -15,6 +15,7 @@ var rootCmd = &cobra.Command{
Use: "tink",
Short: "tinkerbell CLI",
PersistentPreRunE: setupClient,
DisableAutoGenTag: true,
Copy link
Member Author

@displague displague Aug 28, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

This prevents the markdown docs from creating datestamp jitter (diffs), should we include and require updated docs/ in each commit.

@displague displague added area/tink-cli kind/documentation Categorizes issue or PR as related to documentation. kind/feature Categorizes issue or PR as related to a new feature. labels Aug 28, 2020
@thebsdbox
Copy link
Contributor

This looks great, does it require any sort of testing?

@displague
Copy link
Member Author

displague commented Aug 28, 2020 •
edited
Loading

@thebsdbox in an idea world, every line of code is tested. In this case, we have no tests for the tink-cli command or any of its existing subcommands.

This CLI is modeled after the Packet CLI which includes some E2E tests. We can use these as examples if we want to require added tests for this merge. https://github.com/packethost/packet-cli/tree/master/tests

@displague
Copy link
Member Author

@thebsdbox Getting me to say the wrong thing is a sure way to get me to do the right thing. Tests added.

thebsdbox
thebsdbox approved these changes Sep 2, 2020
View reviewed changes
Copy link
Contributor

@thebsdbox thebsdbox left a comment

Choose a reason for hiding this comment

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

Awesome, this looks great :-)

@thebsdbox thebsdbox merged commit c57d19b into tinkerbell:master Sep 3, 2020
@displague displague deleted the docs-command branch September 5, 2020 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thebsdbox thebsdbox thebsdbox approved these changes

Assignees
No one assigned
Labels
area/tink-cli kind/documentation Categorizes issue or PR as related to documentation. kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@displague @thebsdbox