Make some command docs clearer and more descriptive #4286

fiatjaf · 2020-12-18T21:05:40Z

I was very confused with these descriptions that said only "a string that represents a psbt" and no examples of what that string looked like. I wanted to know if it was a base64 string or what (although maybe someone experienced with PSBT stuff would know already), so I made these things more explicit.

fiatjaf · 2020-12-18T23:39:39Z

Made some similar changes in many other command docs with the intent of making them clearer and more descriptive.

doc/lightning-sendpsbt.7.md

doc/lightning-getinfo.7.md

doc/lightning-listnodes.7.md

ZmnSCPxj · 2020-12-21T02:20:38Z

Suggest mergning first and third commit into a single commit --- you could use git rebase -i.

fiatjaf · 2020-12-21T13:15:08Z

Merged everything into a single commit.

doc/lightning-getinfo.7.md

niftynei

Everything except the PSBT change is a NACK from me. You're removing important type information from these descriptions (string, integer, etc)

niftynei · 2020-12-21T21:30:04Z

doc/lightning-ping.7.md

@@ -11,7 +11,7 @@ DESCRIPTION

 The **ping** command checks if the node with *id* is ready to talk. It accepts the following parameters:

- *id*: A string that represents the node id;
+- *id*: Hex-encoded pubkey of the target node;


Suggested change

- *id*: Hex-encoded pubkey of the target node;

- *id*: A string, hex-encoded pubkey of the target node;

How can a "hex-encoded something" not be a string?

niftynei · 2020-12-21T21:30:34Z

doc/lightning-ping.7.md

@@ -33,7 +33,7 @@ RETURN VALUE

 On success, the command will return an object with a single string.

- *totlen*: A string that represents the answer length of the reply message (including header)
+- *totlen*: The length of the reply message (including header)


Suggested change

- *totlen*: The length of the reply message (including header)

- *totlen*: A string, length of the reply message (including header)

This is not a string. See example below.

Good point.

Suggested change

- *totlen*: The length of the reply message (including header)

- *totlen*: Integer. The length of the reply message (including header)

niftynei · 2020-12-21T21:30:51Z

doc/lightning-listtransactions.7.md

@@ -26,21 +26,21 @@ RETURN VALUE

 On success, the command will return a list of transactions, each object represents a transaction, with the following details:

- *hash*: A string that represents the hash of transaction, which the caller can use to find it on the blockchain.


You're stripping the type information from these descriptions. NACK.

The types were wrong in many places. They said "a string that represents whatever" a million times, but very often the type was actually a number. And there was no reference whatsoever to what was the encoding or format of the "string".

What I tried to do was to make the descriptions more readable and more helpful, but not necessarily containing all the possible details.

I actually think I should have removed more of these types, because there are examples right below these descriptions and in the examples the type is much clearer.

The types were wrong in many places.

If the types are wrong, they should be fixed.

but not necessarily containing all the possible details.

The point of documentation is to be both clear and contain all relevant details.

The relevant details are in the examples.

Co-authored-by: Christian Decker <decker.christian@gmail.com>

fiatjaf · 2020-12-24T18:01:43Z

You're really not going to merge this even though it is a clear and obvious improvement just because I've removed some "string" notes from fields that are obviously strings and other "string" notes from fields that are integers?

fiatjaf · 2020-12-24T18:33:39Z

None of the other command docfiles files have this "type information" @niftynei wants so much, not even those of her own authorship. Most don't even have examples like the ones I've modified have.

niftynei · 2020-12-24T19:12:39Z

Hi @fiatjaf. It seems your latest comments are missing some context, namely that you'd like to know what it would take for me to change my NACK to an ACK.

As it stands currently, this PR removes type information from the description fields of the RPC documentation. Restoring the type information to the descriptions would be an ACK from me.

fiatjaf · 2020-12-26T18:50:58Z

You are being unreasonable and ignoring my comments above, but fine. I don't know why I should care about making these docs better anyway.

fiatjaf · 2021-01-07T02:16:38Z

I'll leave this open in case someone wants to build on it.

rustyrussell · 2021-01-09T04:33:16Z

But @niftynei is right. JSON coders expect type info. I like the clarification, but dislike the removal.

We could formalize these into a format like "fieldname: type. Description...".

vincenzopalazzo · 2021-03-22T14:55:38Z

I'm sorry if I jump in the game late, I think that some errors in these docs are my fault, and I'm sorry for that. But it is not easy make check when you need a screen reader to read the documents.

None of the other command docfiles files have this "type information"

I added the documentation of these commands with JSON examples with a point to make the description of the command easy
also for people out of this world. I proposed this format with this issue #3874

As Rusty calls the JSON coders :-D care only about the JSON format, but also sometimes the JSON is more readable for the people with no experience in this world or with some problems like dyslexia. So, I would be happy to have some redundancy inside the documentation such as have some obvious descriptions.

We could formalize these into a format like "fieldname: type. Description...".

Concept ack.

I agree to have a standard in the documentation description.

niftynei · 2021-05-05T22:20:04Z

Closing in favor of #4501

fiatjaf changed the title ~~make base64 encoding clear in PSBT docs.~~ Make some command docs clearer and more descriptive Dec 18, 2020

ZmnSCPxj reviewed Dec 20, 2020

View reviewed changes

doc/lightning-sendpsbt.7.md Outdated Show resolved Hide resolved

doc/lightning-getinfo.7.md Outdated Show resolved Hide resolved

doc/lightning-getinfo.7.md Outdated Show resolved Hide resolved

doc/lightning-listnodes.7.md Outdated Show resolved Hide resolved

clarify property descriptions and examples in many command docs.

01c111c

fiatjaf force-pushed the psbt-doc-clarify branch from 750d8a0 to 01c111c Compare December 21, 2020 13:15

cdecker reviewed Dec 21, 2020

View reviewed changes

doc/lightning-getinfo.7.md Outdated Show resolved Hide resolved

niftynei suggested changes Dec 21, 2020

View reviewed changes

fix grammar.

5242e1b

Co-authored-by: Christian Decker <decker.christian@gmail.com>

fiatjaf closed this Dec 26, 2020

fiatjaf reopened this Jan 7, 2021

rustyrussell mentioned this pull request May 1, 2021

JSON schemas in doc/ (only some commands so far) #4501

Merged

niftynei closed this May 5, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make some command docs clearer and more descriptive #4286

Make some command docs clearer and more descriptive #4286

fiatjaf commented Dec 18, 2020

fiatjaf commented Dec 18, 2020

ZmnSCPxj commented Dec 21, 2020

fiatjaf commented Dec 21, 2020

niftynei left a comment

niftynei Dec 21, 2020

fiatjaf Dec 21, 2020

rustyrussell Jan 9, 2021

niftynei Dec 21, 2020

fiatjaf Dec 21, 2020

niftynei Dec 21, 2020 •

edited

Loading

niftynei Dec 21, 2020

fiatjaf Dec 21, 2020 •

edited

Loading

niftynei Dec 21, 2020

fiatjaf Dec 21, 2020

fiatjaf commented Dec 24, 2020

fiatjaf commented Dec 24, 2020

niftynei commented Dec 24, 2020 •

edited

Loading

fiatjaf commented Dec 26, 2020

fiatjaf commented Jan 7, 2021

rustyrussell commented Jan 9, 2021

vincenzopalazzo commented Mar 22, 2021

niftynei commented May 5, 2021

	- id: Hex-encoded pubkey of the target node;
	- id: A string, hex-encoded pubkey of the target node;

	- totlen: The length of the reply message (including header)
	- totlen: A string, length of the reply message (including header)

	- totlen: The length of the reply message (including header)
	- totlen: Integer. The length of the reply message (including header)

		@@ -26,21 +26,21 @@ RETURN VALUE

		On success, the command will return a list of transactions, each object represents a transaction, with the following details:

		- hash: A string that represents the hash of transaction, which the caller can use to find it on the blockchain.

Make some command docs clearer and more descriptive #4286

Make some command docs clearer and more descriptive #4286

Conversation

fiatjaf commented Dec 18, 2020

fiatjaf commented Dec 18, 2020

ZmnSCPxj commented Dec 21, 2020

fiatjaf commented Dec 21, 2020

niftynei left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

niftynei Dec 21, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fiatjaf Dec 21, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fiatjaf commented Dec 24, 2020

fiatjaf commented Dec 24, 2020

niftynei commented Dec 24, 2020 • edited Loading

fiatjaf commented Dec 26, 2020

fiatjaf commented Jan 7, 2021

rustyrussell commented Jan 9, 2021

vincenzopalazzo commented Mar 22, 2021

niftynei commented May 5, 2021

niftynei Dec 21, 2020 •

edited

Loading

fiatjaf Dec 21, 2020 •

edited

Loading

niftynei commented Dec 24, 2020 •

edited

Loading