Skip to content

Implement help <method-name>#300

Open
liamgilligan wants to merge 1 commit into
ConsensusJ:masterfrom
liamgilligan:walletd-help-implementation
Open

Implement help <method-name>#300
liamgilligan wants to merge 1 commit into
ConsensusJ:masterfrom
liamgilligan:walletd-help-implementation

Conversation

@liamgilligan

Copy link
Copy Markdown
Contributor

PR for #260

Currently filler text is being used, tests have been written/modified to account for new help behavior.

@msgilligan msgilligan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I know this is a draft, but I have a couple suggestions

@msgilligan msgilligan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Additional comment.

@msgilligan

Copy link
Copy Markdown
Member

@liamgilligan See PR #303

@msgilligan

msgilligan commented Jan 26, 2026

Copy link
Copy Markdown
Member

Please rebase now that PR #303 is merged. Also see WIP PR #309.

We should add a JsonRpcHelp (or similar name) class a new .help subpackage of o.c.jsonrpc. Because this module is currently JDK 11, we can't use record, but we can create a record-like class and convert to a record for 0.8.0.

@liamgilligan
liamgilligan force-pushed the walletd-help-implementation branch from 4b0f436 to c6e6377 Compare January 30, 2026 01:24
@liamgilligan
liamgilligan force-pushed the walletd-help-implementation branch from c6e6377 to d7729fa Compare February 5, 2026 22:43

@msgilligan msgilligan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A few requested changes...

@liamgilligan
liamgilligan force-pushed the walletd-help-implementation branch 4 times, most recently from 3f149c6 to f57f010 Compare February 27, 2026 02:03
@liamgilligan
liamgilligan marked this pull request as ready for review February 27, 2026 02:05
@msgilligan
msgilligan force-pushed the walletd-help-implementation branch from f57f010 to 65101e1 Compare March 5, 2026 03:31

@msgilligan msgilligan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good. I have a few requests.

@msgilligan

Copy link
Copy Markdown
Member

See also PR #317 which adds a commit on top of this PR.

Added short summary and detailed help text, added and updated tests, and
added javadoc and copyright headers when applicable.
@liamgilligan
liamgilligan force-pushed the walletd-help-implementation branch from b7b198f to b08f6b3 Compare March 29, 2026 17:36
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