Feature: generate man pages #8525

mohamedawnallah · 2024-03-06T15:37:16Z

Change Description

Generate man pages automatically for both lncli and lnd commands and move the artifacts man pages lncli.1 and lnd.1 to /usr/local/share/man/man1 directory that process happens while running make install command.

Fixes #5605
Replaces #7424

Steps to Test

Steps for reviewers to follow to test the change.

Pull Request Checklist

Testing

Your PR passes all CI checks.
Tests covering the positive and negative (error paths) are included.
Bug fixes contain tests triggering the bug to prevent regressions.

Code Style and Documentation

The change obeys the Code Documentation and Commenting guidelines, and lines wrap at 80.
Commits follow the Ideal Git Commit Structure.
Any new logging statements use an appropriate subsystem and logging level.
Any new lncli commands have appropriate tags in the comments for the rpc in the proto file.
There is a change description in the release notes, or [skip ci] in the commit message for small changes.

📝 Please see our Contribution Guidelines for further guidance.

coderabbitai · 2024-03-06T15:37:22Z

Important

Auto Review Skipped

Auto reviews are limited to the following labels: llm-review. Please add one of these labels to enable auto reviews.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository.

To trigger a single review, invoke the @coderabbitai review command.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>.
- Generate unit-tests for this file.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai generate unit tests for this file.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai generate interesting stats about this repository and render them as a table.
- @coderabbitai show all the console.log statements in this repository.
- @coderabbitai read src/utils.ts and generate unit tests.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
The JSON schema for the configuration file is available here.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

ellemouton

Nice! thanks for this.

Btw - maybe add to the PR description that this replaces #7424

cmd/lncli/commands.go

Makefile

ellemouton · 2024-03-07T08:33:53Z

docs/release-notes/release-notes-0.18.0.md

@@ -188,6 +188,9 @@
  [Deprecate bumpclosefee for bumpforceclosefee and add `max_fee_rate` option
   to `closechannel` cmd](https://github.com/lightningnetwork/lnd/pull/8350).

+* Generate man pages automatically using `lncli generatemanpage` command for
+  both `lncli` and `lnd` commands when running `make install` in the Makefile (https://github.com/lightningnetwork/lnd/pull/8525).


nit: see how the linking is done in other entries here. Make some part of the text clickable instead of just giving the raw link

Thanks! Addressed

ellemouton

see my comment re swallowing the error warning with some bash trickery

mohamedawnallah · 2024-03-11T08:35:26Z

Thanks again @ellemouton for your review. I've addressed all your comments!

ellemouton

Thanks @mohamedawnallah ,

I think we should instead put the script in a dedicated bash script & then call it from the makefile. That way we can reduce the logs printed out too.

So I suggest converting the makefile method to:

manpages:
	@$(call print, "Generating man pages lncli.1 and lnd.1.")
	./scripts/gen_man_pages.sh $(DESTDIR) $(PREFIX)

and then add a gen_man_pages.sh file to the scripts folder with the following contents:

#!/bin/bash

# Usage: ./gen_man_pages.sh DESTDIR PREFIX

DESTDIR="$1"
PREFIX="$2"

# Check if lncli is installed.
if ! command -v lncli &> /dev/null
then
    echo "lncli could not be found. Please install lncli before running this script."
    exit 1
fi

# Ignore warnings regarding HTMLBlock detection in go-md2man package
# since using "<...>" is part of our docs.
lncli generatemanpage 2>&1 | grep -v "go-md2man does not handle node type HTMLSpan" || true

echo "Installing man pages to $DESTDIR$PREFIX/share/man/man1."
install -m 644 lnd.1 "$DESTDIR$PREFIX/share/man/man1/lnd.1"
install -m 644 lncli.1 "$DESTDIR$PREFIX/share/man/man1/lncli.1"

let me know what you think :)

ellemouton · 2024-03-11T11:44:36Z

docs/release-notes/release-notes-0.18.0.md

+* [Generate man pages automatically using `lncli generatemanpage` command
+  for both `lncli` and `lnd` commands](https://github.com/lightningnetwork/lnd/pull/8525) when running `make install` in the Makefile.


pls try wrap this to 80 chars 🙏

Thanks! Addressed

ellemouton

ah - one other thing.

Currently, if you run make manpages, it builds the lnd.1 and lncli. pages in the current directory & then I think install copies them over to the man directory. We should delete them from the current directory.

mohamedawnallah · 2024-03-11T12:18:25Z

Thanks @ellemouton for your feedback. I agree with isolating the man pages script into an isolated bash script it is a much better solution. I've addressed all of your feedback!

ellemouton

@mohamedawnallah - did you test this?

make manpages
 Generating man pages lncli.1 and lnd.1.
./scripts/gen_man_pages.sh  /usr/local
make: ./scripts/gen_man_pages.sh: Permission denied
make: *** [manpages] Error 1

you need to make sure that the new bash script file is executable.

In general, pls always make sure to tests your changes & also to wait for the CI to be green (besides some flakes) before re-requesting review 🙏 😊

mohamedawnallah · 2024-03-11T12:30:57Z

I'm running on zsh on mac I though that was the problem 😁. But you're alright I should have tested it locally before pushing it 👍

mohamedawnallah · 2024-03-11T12:31:24Z

In general, pls always make sure to tests your changes & also to wait for the CI to be green (besides some flakes) before re-requesting review 🙏 😊

Acknowledged 👍

ellemouton

LGTM! Great work @mohamedawnallah 🙏

One last micro nit :) otherwise - LGTM

Makefile

guggero

Very nice. Looks good to me, one comment about the Makefile.

Makefile

guggero

Final nit, otherwise LGTM 🎉

guggero · 2024-03-13T17:07:47Z

Makefile

-#? install: Build and install lnd and lncli binaries, place them in $GOPATH/bin
-install:
+#? install: Build and install lnd and lncli binaries, place them in $GOPATH/bin.
+install-binaries:


nit: the help hint for this make goal is now outdated. Also, looking at all other help hints, we don't use full stops at the end of sentences everywhere else, so let's make this and the ones below consistent with the rest of the file.

mohamedawnallah requested a review from ellemouton as a code owner March 6, 2024 15:37

ellemouton reviewed Mar 7, 2024

View reviewed changes

mohamedawnallah force-pushed the generate-man-pages branch 2 times, most recently from e1da675 to 45c7cc3 Compare March 9, 2024 21:56

mohamedawnallah requested a review from ellemouton March 11, 2024 00:52

ellemouton reviewed Mar 11, 2024

View reviewed changes

mohamedawnallah force-pushed the generate-man-pages branch from 45c7cc3 to 3727582 Compare March 11, 2024 08:30

mohamedawnallah requested a review from ellemouton March 11, 2024 08:35

ellemouton reviewed Mar 11, 2024

View reviewed changes

ellemouton requested changes Mar 11, 2024

View reviewed changes

mohamedawnallah force-pushed the generate-man-pages branch 2 times, most recently from e14be78 to 18bfb77 Compare March 11, 2024 12:17

mohamedawnallah requested a review from ellemouton March 11, 2024 12:20

ellemouton requested changes Mar 11, 2024

View reviewed changes

mohamedawnallah force-pushed the generate-man-pages branch from 18bfb77 to a77f3db Compare March 11, 2024 12:37

ellemouton approved these changes Mar 11, 2024

View reviewed changes

Makefile Outdated Show resolved Hide resolved

mohamedawnallah force-pushed the generate-man-pages branch from a77f3db to 1701c57 Compare March 11, 2024 13:46

guggero self-requested a review March 11, 2024 13:58

saubyk assigned mohamedawnallah Mar 11, 2024

saubyk added the lncli label Mar 11, 2024

saubyk added this to the v0.18.0 milestone Mar 11, 2024

guggero reviewed Mar 13, 2024

View reviewed changes

Makefile Show resolved Hide resolved

mohamedawnallah force-pushed the generate-man-pages branch from 1701c57 to 8950917 Compare March 13, 2024 15:42

guggero approved these changes Mar 13, 2024

View reviewed changes

mohamedawnallah force-pushed the generate-man-pages branch from 09dc489 to bec5764 Compare March 13, 2024 19:18

mohamedawnallah added 3 commits March 13, 2024 21:18

scripts: add gen_man_pages.sh script

dfd1636

lncli+makefile: generate man pages automatically for lncli and lnd

158b802

docs: update release-notes-0.18.0

fb5b425

mohamedawnallah force-pushed the generate-man-pages branch from bec5764 to fb5b425 Compare March 13, 2024 19:18

guggero merged commit 73058c7 into lightningnetwork:master Mar 14, 2024
26 of 27 checks passed

carlaKC mentioned this pull request Mar 15, 2024

[bug]: Manpages Script breaks Dockerfiles #8557

Closed

mohamedawnallah deleted the generate-man-pages branch March 31, 2024 12:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature: generate man pages #8525

Feature: generate man pages #8525

mohamedawnallah commented Mar 6, 2024 •

edited

coderabbitai bot commented Mar 6, 2024

Auto Review Skipped

Chat

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configration File (`.coderabbit.yaml`)

CodeRabbit Discord Community

ellemouton left a comment

ellemouton Mar 7, 2024

mohamedawnallah Mar 9, 2024

ellemouton left a comment

mohamedawnallah commented Mar 11, 2024

ellemouton left a comment

ellemouton Mar 11, 2024

mohamedawnallah Mar 11, 2024

ellemouton left a comment

mohamedawnallah commented Mar 11, 2024

ellemouton left a comment

mohamedawnallah commented Mar 11, 2024

mohamedawnallah commented Mar 11, 2024

ellemouton left a comment

guggero left a comment

guggero left a comment

guggero Mar 13, 2024

		* [Generate man pages automatically using `lncli generatemanpage` command
		for both `lncli` and `lnd` commands](https://github.com/lightningnetwork/lnd/pull/8525) when running `make install` in the Makefile.

Feature: generate man pages #8525

Feature: generate man pages #8525

Conversation

mohamedawnallah commented Mar 6, 2024 • edited

Change Description

Steps to Test

Pull Request Checklist

Testing

Code Style and Documentation

coderabbitai bot commented Mar 6, 2024

Auto Review Skipped

Chat

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configration File (.coderabbit.yaml)

CodeRabbit Discord Community

ellemouton left a comment

Choose a reason for hiding this comment

ellemouton Mar 7, 2024

Choose a reason for hiding this comment

mohamedawnallah Mar 9, 2024

Choose a reason for hiding this comment

ellemouton left a comment

Choose a reason for hiding this comment

mohamedawnallah commented Mar 11, 2024

ellemouton left a comment

Choose a reason for hiding this comment

ellemouton Mar 11, 2024

Choose a reason for hiding this comment

mohamedawnallah Mar 11, 2024

Choose a reason for hiding this comment

ellemouton left a comment

Choose a reason for hiding this comment

mohamedawnallah commented Mar 11, 2024

ellemouton left a comment

Choose a reason for hiding this comment

mohamedawnallah commented Mar 11, 2024

mohamedawnallah commented Mar 11, 2024

ellemouton left a comment

Choose a reason for hiding this comment

guggero left a comment

Choose a reason for hiding this comment

guggero left a comment

Choose a reason for hiding this comment

guggero Mar 13, 2024

Choose a reason for hiding this comment

mohamedawnallah commented Mar 6, 2024 •

edited

CodeRabbit Configration File (`.coderabbit.yaml`)