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

pyln_client: (re-)adds method description to usage via docstring #7680

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

m-schmoock
Copy link
Collaborator

@m-schmoock m-schmoock commented Sep 19, 2024

The extended usage information (long_description) has been removed a while ago without proper replacement. ( commit 2ff3e55 Date: Mon Jul 29 17:58:43 2024 -0700 )

This PR does the following:

  • adds a optional description parameter to a plugin method via docstring that will (if set) be appended with a newline following the generated usage argument list.
  • adds method usage to plugin.print_usage()
  • adds a testcase that checks if extended usage information is passed to manifest json and print_usage

Things to think about

  • Maybe we want to add an optional getmanifest json description field and use that instead of internally appending it with newline to the usage

@m-schmoock
Copy link
Collaborator Author

@cdecker Another option would be to reintroduce the description as a separate decorator to the method but do the same internal handling (appending to usage and passing to manifest json). Maybe that looks nicer in plugin code

@rustyrussell
Copy link
Contributor

We had to fix it quickly for the release, BUT the correct answer is to use the docstring! We already introspect, so accessing doc is easy (which always exists, but is None for no doc string).

@rustyrussell rustyrussell added this to the v24.11 milestone Sep 20, 2024
The old `long_description` was removed and deprecated a while ago
without adding a proper replacement for plugin developers.
The getmanifest JSON that was to be used for that only knows `name` and `usage`.

This PR adds an optional `description` parameter that will be filled
with the methods docstring `__doc__` (if set).

Example:

    @p.method("example")
    def some_method(...)
        """some description"""
        ...

Changelog-Add: optional description paramter to Plugin.Method
@m-schmoock
Copy link
Collaborator Author

We had to fix it quickly for the release, BUT the correct answer is to use the docstring! We already introspect, so accessing doc is easy (which always exists, but is None for no doc string).

Thanks for the clarification. I have been away from the project for a while as you know ;)

I rewrote the PR to use python docstring doc (if set) as the description.

@m-schmoock
Copy link
Collaborator Author

m-schmoock commented Sep 20, 2024

We may want to extend the JSON schema to have the description as a separate field (and not internally newline concatenated to the usage). What do you think?

@m-schmoock m-schmoock changed the title pyln_client: (re-)adds method description to usage pyln_client: (re-)adds method description to usage via docstring Sep 20, 2024
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