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

Implement automatic API documentation for the LCD #2179

Closed
2 of 4 tasks
faboweb opened this issue Aug 29, 2018 · 15 comments
Closed
2 of 4 tasks

Implement automatic API documentation for the LCD #2179

faboweb opened this issue Aug 29, 2018 · 15 comments
Assignees
Labels
T:Docs Changes and features related to documentation.
Milestone

Comments

@faboweb
Copy link
Contributor

faboweb commented Aug 29, 2018

Summary

The Swagger REST documentation is updated manually and in 90% of the cases out of sync. Let's implement a process to updated the documentation automatically. I think we have this for Tendermint already.

Proposal

Investigate https://github.com/swaggo/swag by taking a simple module and adding annotations and generating a Swagger doc for it. If it works, perform this for all handlers and create the necessary scripts/CI tasks needed to automate and document it.


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@fedekunze
Copy link
Collaborator

@faboweb it's being implemented on #2177

@faboweb
Copy link
Contributor Author

faboweb commented Aug 29, 2018

Amazing!

@faboweb faboweb closed this as completed Aug 29, 2018
@jackzampolin jackzampolin reopened this Aug 31, 2018
@jackzampolin
Copy link
Member

We are going with swagger documentation. The Bianje LCD PR implements this. #2199

@faboweb
Copy link
Contributor Author

faboweb commented Sep 3, 2018

As far as I understand #2199 this is not a automatically generated documentation. Am I wrong?

@jackzampolin
Copy link
Member

@faboweb That PR does have auto generated docs for the LCD implementation

@faboweb
Copy link
Contributor Author

faboweb commented Sep 5, 2018

Then I didn't understand the PR correctly. I thought this just generates a visual documentation from a static swagger file. Feel free to close this issue if I am wrong.

@jackzampolin
Copy link
Member

My understanding of the PR is that it generates the swagger file from the golang code using https://github.com/swaggo/swag. So this would be autogenerated documentation unless I'm misreading things...

@fedekunze fedekunze added T:Docs Changes and features related to documentation. API labels Sep 29, 2018
@jackzampolin
Copy link
Member

We have switched to swagger. I'm going to go ahead and close this issue.

@alexanderbez
Copy link
Contributor

alexanderbez commented Aug 26, 2019

Reopening this issue (not sure why it was closed?). Taking a look at https://github.com/swaggo/swag this seems to be what we need. I haven't tested this yet, but ideally we'd annotate each handler and have some sort of script that gathers and generates a single doc file which we use.

@tnachen I see this as high priority because Swagger is often a big point for us in terms of keeping it up to date.

@alexanderbez
Copy link
Contributor

Assigning this to myself and will be working on the handlers piecemeal.

@abelliumnt
Copy link

abelliumnt commented Oct 17, 2019

Before I created PR #2199, I had investigated https://github.com/swaggo/swag. At that time, swag tool has some defects. It can't generate right swagger yaml files if the data structure is too complicated, such as array of struct. So I had to choose a manual way to generate swagger yaml.
Now you'd better evaluate if these defects are still there.

@alexanderbez
Copy link
Contributor

@HaoyangLiu do you have concrete examples? We are able to annotate return types that have arrays (e.g. validators)

@abelliumnt
Copy link

The main defects I found one years ago:

  • Doesn't support array of struct
  • Requires all handler must be locate in a root fold, which mean we have to refactor code structure.

Now if the above defects are solved or some defects are acceptable, then you can start to integrate swag with gaiacli

@alexanderbez
Copy link
Contributor

alexanderbez commented Oct 18, 2019

Requires all handler must be locate in a root fold, which mean we have to refactor code structure.

This isn't true anymore I think. So far it works and we have handlers implemented in modules.

Doesn't support array of struct

Also not true anymore I believe.

That said, we are having some issues with the types being rendered. If you checkout #5038 and run make update-swagger-docs you'll get new results each time. Still debugging this one.

@fedekunze fedekunze mentioned this issue Nov 13, 2019
4 tasks
@tac0turtle
Copy link
Member

gRPC swagger gen does this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T:Docs Changes and features related to documentation.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants