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

Add RPC documentation generator #291

Merged
merged 8 commits into from
Nov 13, 2024

Conversation

chenyukang
Copy link
Collaborator

@chenyukang chenyukang commented Oct 27, 2024

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 Fix the issues of strip and merge desc for code block GREsau/schemars#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
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

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

Additional details and impacted files
@@            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 Coverage Δ
unittests 44.45% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@contrun
Copy link
Collaborator

contrun commented Oct 28, 2024

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
Copy link
Collaborator Author

chenyukang commented Oct 28, 2024

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.

@quake quake merged commit 51ce132 into nervosnetwork:main Nov 13, 2024
16 checks passed
@quake quake mentioned this pull request Nov 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.

4 participants