Skip to content

Conversation

@mtfurlan
Copy link
Contributor

@mtfurlan mtfurlan commented May 13, 2020

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
Copy link
Contributor

mislav commented May 13, 2020

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

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

@ryanblakeley
Copy link

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

git clone [email protected]:mtfurlan/cli.git github-cli
cd github-cli
gco docs/manpage
make
gh version
man gh

@mtfurlan
Copy link
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
Copy link
Contributor

@vilmibm vilmibm left a comment

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.

@mtfurlan
Copy link
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
Copy link
Contributor

vilmibm commented May 19, 2020

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).

Copy link
Contributor

@vilmibm vilmibm left a comment

Choose a reason for hiding this comment

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

thank you much!

Copy link
Contributor

@mislav mislav left a comment

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

@vilmibm
Copy link
Contributor

vilmibm commented May 19, 2020

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
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
Copy link
Contributor

mislav commented May 20, 2020

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
Copy link
Contributor

vilmibm commented May 20, 2020

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

@vilmibm
Copy link
Contributor

vilmibm commented May 20, 2020

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