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 specific docs for account public keys #232

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

segfaultxavi
Copy link
Collaborator

There are conditions for this field's validity which
need to be pointed out.

There are conditions for this field's validity which
need to be pointed out.
@segfaultxavi segfaultxavi requested a review from Vektrat October 6, 2020 10:06
@segfaultxavi segfaultxavi added the documentation Improvements or additions to documentation label Oct 6, 2020
publicKeyHeight:
$ref: "../../../schemas/Height.yml"
allOf:
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the allOf: @segfaultxavi ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's an ugly workaround suggested by the OpenAPI Initiative guys themselves.
$ref nodes cannot have descriptions, and we cannot add the description to Height.yml because a publicKeyHeight is not the same thing as a addressHeight, even though both are Heights.
This is solved in the next version of OpenAPI, but it will take a while for all tools to support it. Meanwhile, the workaround is to add an allOf, so the $ref is merged with the description.
I tested the generated documentation and it looks as intended.

I also created an issue so we add these docs in all the missing places: #233

Copy link
Contributor

Choose a reason for hiding this comment

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

If it doesn't break the client generators all good

https://github.com/nemtech/symbol-openapi-generator

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You're right, this breaks the generators. I've spent a few hours looking into it but this seems like a long-standing issue in openapi-generator.
Let's park this PR and hope that OpenAPI 3.1 is better supported...

@segfaultxavi segfaultxavi marked this pull request as draft October 26, 2020 10:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants