nervosnetwork / fiber

21 stars 11 forks source link

Add RPC documentation generator #291

Closed chenyukang closed 1 week ago

chenyukang commented 4 weeks ago

Some other ways I have tried:

  1. use json format from cargo doc, but it's only available for nightly version of rustc, and we also need to convert from json format to markdown format.
  2. use https://github.com/GREsau/schemars and add JsonSchema for all data structures which need to be listed in markdown, I think it's an invasive change, and there are some trivial issues need to be resolved, such as https://github.com/GREsau/schemars/pull/251

The current simple but dirty solution https://github.com/chenyukang/rpc-doc-gen is based on syn crate, which may have some pitfalls but I think we can improve it later.

codecov-commenter commented 4 weeks ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 44.45%. Comparing base (f61ee09) to head (7466c23). Report is 2 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #291 +/- ## ========================================== - Coverage 44.46% 44.45% -0.01% ========================================== Files 44 44 Lines 28006 27999 -7 ========================================== - Hits 12453 12448 -5 + Misses 15553 15551 -2 ``` | [Flag](https://app.codecov.io/gh/nervosnetwork/fiber/pull/291/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=nervosnetwork) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/nervosnetwork/fiber/pull/291/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=nervosnetwork) | `44.45% <ø> (-0.01%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=nervosnetwork#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

contrun commented 4 weeks ago

Thanks for tackling this. It is a life quality improvement. I skimmed through the generator implementation.

Current implementation didn't consider foreign types in enum/struct fields, right? For example, the Multiaddr parameter of the RPC connect_peer is not documented (if we are using cargo doc to generate json files, I would expect we have foreign type information).

Another thing users want to know is how are these types serialized and deserialized. For example, if I want to pass a Multiaddr to the RPC server, what is the appropriate format? I don't think it is quite possible for us to automatically document that, is it?

A small problem of the generated document is that it seems the type name of any Option fields are quoted, while other type names are not. For example, below the type name Hash256 is not quoted but Option<u64> is.

channel_id - Hash256, Channel ID for the CKB payment. tlc_id - Option<u64>, TLC ID for the CKB payment.

chenyukang commented 4 weeks ago

yes, there are some improvements that need to be resolved:

  1. we need to analyze all the code base, I already tried it but need some tweak for the generator to dump only the related types with RPC.
  2. all types should be quoted, and Option<u64> will be better in the format null | u64, and Option<SomeStruct> will be in format null | SomeStruct.