-
Notifications
You must be signed in to change notification settings - Fork 138
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
CBG-4143 add redocly targets for capella #7044
Conversation
Redocly previews |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great, thanks Tor.
This means that we (docs) could potentially submit PRs against these -capella files for some of the other wording tweaks that @idulo @ElliotFrancisHunter identified.
I think that means we'd probably want a public-capella.yml too (though that would initially be identical, and we'd tweak the wordings from there 👍)
docs/api/admin-capella.yaml
Outdated
paths: | ||
'/{db}/_session': | ||
$ref: './paths/admin/db-_session.yaml' | ||
x-capella: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good just a minor point.
How come this has been added here but nowhere else?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought this would work to use filter-in
but it does not because filter-in only works if a property is defined. I think it'd be too much to remember to define a property on every operation.
This was an artifact from a previous experiment. Any properties with x-
are ignored by openapi and redocly unless redocly gives them special meanings.
I definitely think we can do some wording tweaks, probably for all the files. I think rarely does Sync Gateway have to be included as a literal, and we could definitely use |
create decorators to do munging in couchbase/docs-capella#58
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great to me 👍
Also tagging @simon-dew @ElliotFrancisHunter for info / if they want to leave a review/questions.
One question inline about public-capella which suggests there's a little tidying to do / or that I'm confused.
It's very helpful to see Redocly generators used in practice, thank you for developing these!
What I * haven't* yet done is to generate the output and compare it to what we generated. I don't think that needs to prevent this going in though -- the basics of what it's doing look sensible, and we will want to iterate a few things in any case
public: | ||
root: "./docs/api/public.yaml" | ||
decorators: | ||
remove-x-internal: on | ||
public-internal: | ||
root: "./docs/api/public.yaml" | ||
public-capella: | ||
root: "./docs/api/public.yaml" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this be public-capella, or is the public-capella.yaml still in this PR now obsolete?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess that question is "Did the filter-out x-capella thing work, or is that a failed experiment?"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public-capella.yaml is dead, thanks for catch.
For filter-out x-capella, it does work, it's in public.yaml
to remove the views APIs. Using filter-out
the property has to be defined, so it can't just filter out everything that doesn't have the property defined. That's why admin and metric use different toplevel yaml files, since they have such limited APIs.
I've added filter-out x-capella preemptively for admin and metric endpoints, since we could filter out parts of a particular operation. We aren't doing that yet but could in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed will merge as-is to keep the status-quo with the current version of docs.
I thought I could use https://redocly.com/docs/cli/decorators/filter-in but I used it only applies to filter out things if a property is defined. Additionally, having the tags for each section that was empty seemed wrong.
I changed these only slightly from non capella variants:
externalDocs
link but this is unusedTo see these docs:
npx @redocly/cli@latest preview-docs admin-capella
andnpx @redocly/cli@latest preview-docs metric-capella
There's a 3.1.10 backport that could be used for the newest release onto capella on https://github.com/couchbase/sync_gateway/tree/CBG-4144. 3.0.8 is the version released on capella but there is no functional api differences.