Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Generate manpages from cobra - #915

#915
Merged
vilmibm merged 10 commits into
cli:mastercli/cli:masterfrom
mtfurlan:docs/manpageCopy head branch name to clipboard
May 20, 2020
Merged

Generate manpages from cobra#915
vilmibm merged 10 commits into
cli:mastercli/cli:masterfrom
mtfurlan:docs/manpageCopy head branch name to clipboard

Conversation

@mtfurlan

@mtfurlan mtfurlan commented May 13, 2020

Copy link
Copy Markdown
Contributor

fixes #900

In it's current state the manpages aren't great, but are technically correct, better than nothing, and will be kept up to date.

@mislav
mislav self-requested a review May 13, 2020 18:33
@mislav

mislav commented May 13, 2020

Copy link
Copy Markdown
Contributor

@mtfurlan Thank you for proposing this; it looks promising!

I will give a more thorough review when I find time 🙇

@ryanblakeley

Copy link
Copy Markdown

what's the way to test this? i took a stab but didn't get a result from man gh.

git clone git@github.com:mtfurlan/cli.git github-cli
cd github-cli
gco docs/manpage
make
gh version
man gh

@mtfurlan

Copy link
Copy Markdown
Contributor Author

@rojobuffalo

make manpages
man ./manpage/gh.1
man ./manpage/gh.whatever.1

They're currently split up so every subcommand gets it's own man page cause that's how cobra does it.

It's not amazing, but it's better than nothing and it's technically correct.

@vilmibm
vilmibm self-requested a review May 19, 2020 15:35

@vilmibm vilmibm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I just have one small nitpick change requested.

Otherwise, this is exciting! I agree that what cobra outputs is not ideal but I'm happy with this as a starting point. I just did a test release and installed a .deb and saw that things were in place.

Thank you for this.

Comment thread cmd/gen-docs/main.go Outdated
@mtfurlan

Copy link
Copy Markdown
Contributor Author

Two other things I now that I'm looking over this again:

  • The archive outputs have a manpage directory, because I was copying the approach used by the exoscale cli.
    Seems a bit weird, but not harmful?
    Is there a better idea?
  • There has to be a way to put the manpages in the homebrew output, but it looks like it might just be copy it somewhere in the install step?
    I can't find an example and I don't have a way to guess and check.

@vilmibm

vilmibm commented May 19, 2020

Copy link
Copy Markdown
Contributor

The archive outputs have a manpage directory, because I was copying the approach used by the exoscale cli.
Seems a bit weird, but not harmful?
Is there a better idea?

eh, I don't have a problem with it. If we determine it's a problem later on we can circle back.

There has to be a way to put the manpages in the homebrew output, but it looks like it might just be copy it somewhere in the install step?
I can't find an example and I don't have a way to guess and check.

I know next to nothing about homebrew. I did set up linuxbrew so I could ensure that the brew packages were upgrading properly but yeah, someone would have to research how brew packages tend to handle manpages. I think it's ok to start with them only installing automatically on linux (it's an incremental upgrade from not having them at all).

@vilmibm vilmibm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

thank you much!

@mislav mislav left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good!

We should also amend our Homebrew formula in goreleaser.yml to include the line

man1.install Dir["manpages/*.1"]

It has also ocurred to me that we probably want make manpages as part of our release process

Comment thread Makefile Outdated
@vilmibm

vilmibm commented May 19, 2020

Copy link
Copy Markdown
Contributor

if someone wants to test out manpage installation via brew on a mac that would be 💖

otherwise I'll find and charge my very unutilized work mac tomorrow and test it

@mtfurlan

Copy link
Copy Markdown
Contributor Author

I figured out how to make goreleaser build the makefiles, and that cobra will make the output dir for us and it doesn't need to be in the makefile.

@mislav

mislav commented May 20, 2020

Copy link
Copy Markdown
Contributor

I've tested the release with goreleaser --skip-validate --skip-publish --rm-dist and Homebrew installation with brew install ./dist/gh.rb (after massaging gh.rb a bit to find release files on local disk rather than on a remote URL) and it all checks out! Thank you @mtfurlan for setting this up and @vilmibm for reviewing ✨

@vilmibm

vilmibm commented May 20, 2020

Copy link
Copy Markdown
Contributor

thank you for the macos test! i was dreading getting my mbp up and running again haha

@vilmibm

vilmibm commented May 20, 2020

Copy link
Copy Markdown
Contributor

thanks @mtfurlan for making this happen and to @mislav for the homebrew and CI stuff!

@vilmibm
vilmibm merged commit e9d6e13 into cli:master May 20, 2020
@probablycorey probablycorey mentioned this pull request May 20, 2020
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.

Generating manpages

4 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.