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

generate man pages for kuma components #2101

Merged
merged 4 commits into from
Jun 24, 2021
Merged

Conversation

tharun208
Copy link
Contributor

@tharun208 tharun208 commented Jun 4, 2021

generate man pages for kuma components

Signed-off-by: Tharun rajendrantharun@live.com

Testing

  • Unit tests
  • E2E tests
  • Manual testing on Universal
  • Manual testing on Kubernetes

@tharun208 tharun208 requested a review from a team as a code owner June 4, 2021 15:20
@tharun208 tharun208 changed the title feat(kumactl) feat(kumactl) generate man pages for cli Jun 4, 2021
generate man pages for the kumactl cli

Signed-off-by: Tharun <rajendrantharun@live.com>
@jpeach
Copy link
Contributor

jpeach commented Jun 6, 2021

A few questions ....

  • What's the use case for this? Would users run kumactl genman | man -l - (note, I had to google that!)? Or would you want to install this as part of a distribution package?
  • Why only kubectl and not all the commands?
  • Would an online reference help?

@tharun208
Copy link
Contributor Author

tharun208 commented Jun 7, 2021

A few questions ....

  • What's the use case for this? Would users run kumactl genman | man -l - (note, I had to google that!)? Or would you want to install this as part of a distribution package?
  • Why only kubectl and not all the commands?
  • Would an online reference help?
  1. Yes, we can add this to be installed as a part of the homebrew kumactl formulae script.
  2. Because kumactl is the only part of homebrew ( maybe we can update the genman command for all kuma components, but generating man pages while installing will be only done for kumactl through the brew formulae script).
  3. The man pages that are generated are not a single file and I think it is not a good idea to keep it on the website.

Need your thoughts for the online reference @jpeach @nickolaev

@jpeach
Copy link
Contributor

jpeach commented Jun 7, 2021

@tharun208 I chatted about this with @bartsmykla and we have 3 kinds of documentation that we want to generate:

  • protobuf API docs
  • markdown CLI usage docs
  • kumactl man pages

We propose that we add a consistent set of makefile targets for generating these, rather than building them into the CLI itself. So you would do

$ DESTDIR=/usr/local/man/man1 make docs/install/man
$ DESTDIR=/usr/share/doc/kuma/proto make docs/install/proto
$ DESTDIR=/usr/share/doc/kuma/cli make docs/install/markdown

Does that meet your needs?

@tharun208
Copy link
Contributor Author

@tharun208 I chatted about this with @bartsmykla and we have 3 kinds of documentation that we want to generate:

  • protobuf API docs
  • markdown CLI usage docs
  • kumactl man pages

We propose that we add a consistent set of makefile targets for generating these, rather than building them into the CLI itself. So you would do

$ DESTDIR=/usr/local/man/man1 make docs/install/man
$ DESTDIR=/usr/share/doc/kuma/proto make docs/install/proto
$ DESTDIR=/usr/share/doc/kuma/cli make docs/install/markdown

Does that meet your needs?

sounds good. But, I am a bit confused about man pages for kumactl. Is there a way to achieve this using the bash script?.
I think the only and easy way to generate it by using the cobra package inbuilt functions.

@jpeach
Copy link
Contributor

jpeach commented Jun 8, 2021

@tharun208 I chatted about this with @bartsmykla and we have 3 kinds of documentation that we want to generate:

  • protobuf API docs
  • markdown CLI usage docs
  • kumactl man pages

We propose that we add a consistent set of makefile targets for generating these, rather than building them into the CLI itself. So you would do

$ DESTDIR=/usr/local/man/man1 make docs/install/man
$ DESTDIR=/usr/share/doc/kuma/proto make docs/install/proto
$ DESTDIR=/usr/share/doc/kuma/cli make docs/install/markdown

Does that meet your needs?

sounds good. But, I am a bit confused about man pages for kumactl. Is there a way to achieve this using the bash script?.
I think the only and easy way to generate it by using the cobra package inbuilt functions.

yeh, you can just write the wrapper in ./tools/docs/manpages.go and then go run that from the build.

@tharun208
Copy link
Contributor Author

@tharun208 I chatted about this with @bartsmykla and we have 3 kinds of documentation that we want to generate:

  • protobuf API docs
  • markdown CLI usage docs
  • kumactl man pages

We propose that we add a consistent set of makefile targets for generating these, rather than building them into the CLI itself. So you would do

$ DESTDIR=/usr/local/man/man1 make docs/install/man
$ DESTDIR=/usr/share/doc/kuma/proto make docs/install/proto
$ DESTDIR=/usr/share/doc/kuma/cli make docs/install/markdown

Does that meet your needs?

sounds good. But, I am a bit confused about man pages for kumactl. Is there a way to achieve this using the bash script?.
I think the only and easy way to generate it by using the cobra package inbuilt functions.

yeh, you can just write the wrapper in ./tools/docs/manpages.go and then go run that from the build.

sure, then can we do all the three documentation implementations in the same pr?

@tharun208 tharun208 changed the title feat(kumactl) generate man pages for cli feat(kumactl) generate man pages for kuma components Jun 16, 2021
Signed-off-by: Tharun <rajendrantharun@live.com>
@tharun208 tharun208 changed the title feat(kumactl) generate man pages for kuma components generate man pages for kuma components Jun 17, 2021
@tharun208
Copy link
Contributor Author

tharun208 commented Jun 17, 2021

@jpeach, do we need the generated man pages also to be part of the codebase?

@jpeach
Copy link
Contributor

jpeach commented Jun 21, 2021

@jpeach, do we need the generated man pages also to be part of the codebase?

I don't know of any reason to want that, do you?

@tharun208
Copy link
Contributor Author

@jpeach, do we need the generated man pages also to be part of the codebase?

I don't know of any reason to want that, do you?

no

@jpeach jpeach self-assigned this Jun 21, 2021
@jpeach
Copy link
Contributor

jpeach commented Jun 23, 2021

Looking at the generated output, the .TH element is all wrong. The format is documented here, so it ought to be something like this:

.nh
.TH kuma\-cp 1 2021-06-01 Kuma "Kuma Manual"

but it gets generated like this:

.nh
.TH kuma\-cp(1)Jun 2021
Auto generated by spf13/cobra

Looks like this is spf13/cobra#1049

tools/docs/generate.go Outdated Show resolved Hide resolved
tools/docs/generate.go Outdated Show resolved Hide resolved
@jpeach
Copy link
Contributor

jpeach commented Jun 23, 2021

Once #2196 lands, could you please rebase? Thanks!

Signed-off-by: Tharun <rajendrantharun@live.com>
@tharun208 tharun208 requested a review from jpeach June 23, 2021 05:11
Copy link
Contributor

@jpeach jpeach left a comment

Choose a reason for hiding this comment

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

This looks great. I'm going to land this without waiting for the cobra upgrade, because that is stuck behind some integration test failures.

@jpeach jpeach merged commit 06236c6 into kumahq:master Jun 24, 2021
@tharun208 tharun208 deleted the feat/man_pages branch June 24, 2021 04:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants