Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Dev Docs: Add 114 Pages Of New/Rewritten RPC Docs #693

Merged
merged 2 commits into from Jan 3, 2015

Conversation

Projects
None yet
2 participants
Contributor

harding commented Dec 25, 2014

Preview: http://dg1.dtrt.org/en/developer-reference#rpc-quick-reference

Note: I plan on merging this January 1st even if it doesn't get any full reviews. Please comment if you oppose that plan. This is a large amount of text and very boring to read, so if nobody has reviewed it within a week, I expect nobody will ever review it. We still display a disclaimer telling users about the high probability of mistakes.

Major Improvements

  • Better formatting---the tabular format names elements, types them, and describes them all in the same location, making the docs easier to read.
  • More information---the previous RPC docs were based on the help text. These docs were rewritten as I read the source code. This provided me much more information and allowed me to write better descriptions.
  • Easier maintainability---the tabular format is much easier to update as Description fields can say "updated in Bitcoin Core n.nn.n" and the Quick Reference section emphesizes recent changes. This makes possible one of my goals: updating the RPC docs throughout the release cycle as changes get merged into Bitcoin Core master.
  • v0.10.0 Ready---the reason I started this work: we now document all the new RPCs and RPC updates from the v0.10 Bitcoin Core branch (except for the hidden RPCs). All examples have been re-run with Bitcoin Core 0.10.0rc1 (2014-12-23), and the example updated if anything changed. I'll keep diffing to make sure nothing changes between that tag and the final release.

Minor (But Fun) Improvements

  • More colorful---all RPC examples now use Pygments syntax highlighting.
  • Less redundant---reading the source code let me see where several RPCs all used the same functions, allowing me write descriptions once in a variable and then use that variable in every RPC description that used the function. I didn't go crazy and variablize everything---just long segments of documentation.
  • Less byte order confusion---we now mention hash byte order wherever it matters (with links to our docs about it), hopefully decreasing confusing among new developers.
  • See also---each RPC ends with a See Also section pointing to other RPCs and reference information to help readers navigate RPCs by relationship.

Not Improved

  • GetBlockTemplate undocumented---instead of trying to document GBT, we point to its Wiki page, BIP22, and BIP23. I will document GBT someday, but it's the largest RPC by a sizable margin and mining stuff is currently low priority for me.

Future Work For Subequent Pulls

  • Describe errors---the human-readable RPC error messages tend to be satisfactorily descriptive, so further documentation of them is low priority to me.
  • Link to various libraries---I plan to check out a few libraries for JSON in general and Bitcoin Core in particular and write some examples for the examples page.
  • Backlink to Bitcoin Core---once 0.10.0 final is released, add links from each RPC's See Also section to its declaration in the source code, or maybe to Wladimir's Doxygen page.
  • Maybe document the new hidden RPCs? Opinions on this welcome: I lean towards not documenting stuff the developers aren't displaying to regular RPC users.

A more straightforward description of the changes can be found in the commit message.

Dev Docs: Add 114 Pages Of New/Rewritten RPC Docs
* All previously-documented RPCs have had their text completely
  rewritten.

* All new RPCs and changed RPCs in Bitcoin Core 0.10.0 have been
  documented, except for hidden RPCs.

* A new RPC "Quick Reference" section has been added to make finding the
  right RPC easier.

* A "See Also" subsection has been added to the end of every RPC
  pointing to other relevant information.

* All previous examples in the RPC section have been re-run and updated
  as necessary.

* Syntax highlighting has been added wherever possible.

* Hash byte order has been specified as RPC byte order everywhere it's
  used in RPCs.
Contributor

saivann commented Dec 29, 2014

I haven't reviewed everything but thus far this looks excellent!

@harding Do you think we could keep the previous menu order?

  • Bitcoin Core APIs
  • - Remote Procedure Calls (RPCs)
  • - - (each RPC)

Less childs in the menu makes it more readable, while at the same time making titles more obvious in the texts by using

tags instead of

tags.

Regarding the partially auto-generated summaries.md file, how about we edit this file manually (like references.md and others) and at the same time avoid duplicating the "assign summary_*" lines in each individual RPC file? We could perhaps make the "check-for-missing-rpc-summaries" rule also check for orphan summaries?

Contributor

harding commented Dec 30, 2014

@saivann thanks!

Re: previous menu order: to draw a quick illustration of current vs new order so I can keep things straight:

master branch                    newrpc branch
~~~~~~~~~~~~                     ~~~~~~~~~~~~~~
## Core APIs                     ## Core APIs
  ### Hash Byte Order             ### Hash Byte Order
  ### JSON-RPC...                 ### Remote...RPCs
  ### Remote...RPCs                #### JSON-RPC...
    #### AddMultiSig...            #### Quick Reference
                                   #### RPCs
                                     ##### AddMultiSig...

I like how putting the JSON-RPC subsection and Quick Reference subsection under the main Remote Procedure Calls section in newrpc keeps the RPC-related meta-information separate from the RPCs themselves but still in the RPC section. In master, this information is separated. I think the increased depth will become more useful when I write docs for the new REST API, which will look something like,

## Core APIs
  ### Hash Byte Order
  ### Remote...RPC
    #### JSON-RPC...
    #### Quick Ref
    #### RPCs
      ...
  ### REST
    #### HTTP REST
    #### Quick Ref
    #### Resources
      ...

I've also tried to make titles more obvious in the individual RPCs by not using subheads or all-bold lines at all. (For example, I used all-italics lines for subsection breaks.) Maybe we could use horizontal rules (<hr>) to break up the text further, or put the whole section in its own div with its own styles for h5 tags?

Long-term (e.g. probably for the 0.11 release), I think we should probably move these Bitcoin Core API docs into a separate document focused on Bitcoin Core administration and programming. If we do that, we can promote the Remote Procedure Calls subsection to h2 and the RPCs subsection to h3, making the individual RPCs back into h4s.


Re: making summaries.md editable. Yeah, there's a lot of trade-offs here and I'm not sure what the best option is. My main goal here is obviously to make sure that if the description gets updated in one place, it gets updated everywhere else it's used. My secondary goal is to make editing the descriptions as easy as possible for new contributors, and it seemed to me that putting the description in the individual RPCs was the easiest way to do that---it put the summary in context and you or I can always run the manual-update command when we merge in the pull.

However, if you think maintaining the summaries in a separate file will be easier, I'll happily update the Makefile rules. Let me know.

Thanks again!

Contributor

saivann commented Dec 30, 2014

@harding Thanks for commenting! Please forget my idea of re-organizing the menu, your plans sound good! In order to make the titles more visible, here's an additional styling change I like in case you like it too:

.toccontent table thead th{
font-weight:normal;
background-color:#f5f5f5;
}

Re: summaries.md, OK let's not waste precious time on this then. Maintaining them in a separate file seemed a bit more consistent to me with how other similar files are managed. Maybe you could add explicit instructions in the error message in the Makefile (see suggestion below)? I think this would prevent the only situation where I see a future contributor could be left confused.

echo "missing summary for $$f , you need to add the summary and run \"make manual-updates\""

@harding harding added the Dev Docs label Dec 30, 2014

Dev Docs: Put RPC Tables In Separate CSS Class
Suggested by Saivann: RPC tables with Name/Type/Presence/Description
format are now in .ntpd CSS class so that we can format them specially.

We also change the style of all dev doc tables to replace bold table
headings (thead) with a double-line border-bottom.  This makes the
theads look less like section headings and prevents us from over-using
bold.

Minor: a Makefile warning message has been made more explicit (also
suggested by Saivann) and another Makefile rule was updated to catch
more broken tables.
Contributor

harding commented Dec 30, 2014

@saivann added commit harding/bitcoin.org@9329c69 and updated preview based on your feedback. Thanks!

As previously announced, in the absence of critical feedback, I plan to merge this branch the morning of January 1st.

@harding harding merged commit 9329c69 into bitcoin-dot-org:master Jan 3, 2015

harding added a commit that referenced this pull request Jan 3, 2015

Contributor

saivann commented Jan 3, 2015

@harding Thanks! And sorry again for my recent unavailability, I'll open issues or pull request if I find anything at a later time.

@harding harding deleted the harding:newrpc branch Feb 25, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment