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.

Sign up for GitHub

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

Jump to bottom

docs: response examples should match API #3840

Open
jcstein opened this issue Oct 11, 2024 · 2 comments · May be fixed by #3848
Open

docs: response examples should match API #3840

jcstein opened this issue Oct 11, 2024 · 2 comments · May be fixed by #3848
Labels
external Issues created by non node team members needs:triage priority:high High priority issue to be prioritized for the current/following sprint

Comments

@jcstein
Copy link
Member

jcstein commented Oct 11, 2024

Problem

From Slack thread

I generated specs using celestia docgen on v0.16.0 and v0.17.1 and added them to docs, but it is reported that the response examples are incorrect on docs still, although the API changed. API spec updates from v0.16.0 and v0.17.1 do not contain correct responses, for at least tx responses:

Source: https://github.com/celestiaorg/celestia-node/releases/tag/v0.16.0

API Response Changes: The return type for SubmitPayForBlob remains unchanged, but most fields in the response will now be omitted. Only the Code, TxHash, and Height fields will be populated, with all other fields set to empty values.

Solution

Modify the docs within celestia-node so that when docs are generated in the CI (building the openrpc.json specs) or manually in CLI, they contain the correct responses
@github-actions github-actions bot added needs:triage external Issues created by non node team members labels Oct 11, 2024
@jcstein
Copy link
Member Author

jcstein commented Oct 11, 2024

Note that these were manually generated because CI did not build the specs for either of the mentioned releases.

@zvolin
Copy link

zvolin commented Oct 12, 2024

I think the reason for that could be because the type hasn't changed on celestia-node side, just now celestia-app responds with the other type that only partially maps to the TxResponse. Docs probably just fill dummy values for the type they see

@renaynay renaynay added the priority:high High priority issue to be prioritized for the current/following sprint label Oct 14, 2024
@walldiss walldiss linked a pull request Oct 14, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
external Issues created by non node team members needs:triage priority:high High priority issue to be prioritized for the current/following sprint
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(docs) update txResponse in docs walldiss/celestia-node
3 participants
@zvolin @renaynay @jcstein