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

feat(rpc): tooling for openrpc spec autogeneration #1283

Merged
merged 7 commits into from
Nov 16, 2022

Conversation

distractedm1nd
Copy link
Collaborator

@distractedm1nd distractedm1nd commented Oct 26, 2022

Closes #1188

First prototype:

  • Example parameter types
  • Relying on explicitly defining default implementations for method discovery
  • Refined metadata
  • Scaffolding for example parameter types (NOTE: finishing this will be its own PR)

Long term(not this PR, issues to be created):

  • Allow for embedded interfaces in nodebuilder Module interfaces
  • Autogenerated docs for methods that return a channel
  • Create Make targets for the code autogeneration
  • Have CI generate the spec and host it for use with playground

@distractedm1nd distractedm1nd added area:rpc kind:feat Attached to feature PRs labels Oct 26, 2022
@distractedm1nd distractedm1nd self-assigned this Oct 26, 2022
@mergify
Copy link

mergify bot commented Oct 26, 2022

⚠️ The sha of the head commit of this PR conflicts with #1195. Mergify cannot evaluate rules on this PR. ⚠️

nodebuilder/share/service.go Show resolved Hide resolved
nodebuilder/share/service.go Show resolved Hide resolved
api/docgen/cmd/docgen.go Outdated Show resolved Hide resolved
api/docgen/cmd/docgen.go Outdated Show resolved Hide resolved
api/docgen/openrpc.go Outdated Show resolved Hide resolved
go.mod Outdated Show resolved Hide resolved
api/docgen/openrpc.go Show resolved Hide resolved
api/docgen/openrpc.go Show resolved Hide resolved
api/docgen/examples.go Outdated Show resolved Hide resolved
nodebuilder/default_services.go Show resolved Hide resolved
Wondertan
Wondertan previously approved these changes Nov 11, 2022
Copy link
Member

@Wondertan Wondertan left a comment

Choose a reason for hiding this comment

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

LGTM. The only concern is bloating of makefile to build docgen binary, that might not have any usage

Makefile Outdated Show resolved Hide resolved
api/docgen/openrpc.go Show resolved Hide resolved
nodebuilder/share/service.go Show resolved Hide resolved
vgonkivs
vgonkivs previously approved these changes Nov 14, 2022
Copy link
Member

@vgonkivs vgonkivs left a comment

Choose a reason for hiding this comment

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

LGTM, but I agree with @Wondertan , we should remove building docgen in makefile

@codecov-commenter
Copy link

Codecov Report

Merging #1283 (b481070) into main (bd13b76) will decrease coverage by 0.20%.
The diff coverage is 25.00%.

@@            Coverage Diff             @@
##             main    #1283      +/-   ##
==========================================
- Coverage   55.06%   54.86%   -0.21%     
==========================================
  Files         178      178              
  Lines       10689    10689              
==========================================
- Hits         5886     5864      -22     
- Misses       4227     4248      +21     
- Partials      576      577       +1     
Impacted Files Coverage Δ
nodebuilder/share/service.go 100.00% <ø> (ø)
nodebuilder/header/service.go 57.14% <25.00%> (ø)
share/eds/byzantine/bad_encoding.go 65.00% <0.00%> (-3.00%) ⬇️
share/eds/byzantine/pb/share.pb.go 32.88% <0.00%> (-1.97%) ⬇️
share/ipld/get.go 91.24% <0.00%> (-1.39%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Wondertan
Wondertan previously approved these changes Nov 15, 2022
@Wondertan
Copy link
Member

@distractedm1nd, small conflict

renaynay
renaynay previously approved these changes Nov 16, 2022
api/docgen/openrpc.go Show resolved Hide resolved
api/docgen/openrpc.go Show resolved Hide resolved
@distractedm1nd distractedm1nd merged commit e21de2c into celestiaorg:main Nov 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:rpc kind:feat Attached to feature PRs
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

add JSON-RPC autogenerated docs
5 participants