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

api: Document /realm/presence API endpoint. #22509

Closed
wants to merge 2 commits into from

Conversation

akashaviator
Copy link
Collaborator

@akashaviator akashaviator commented Jul 17, 2022

This documents the /realm/presence API endpoint and fixes a bug in render_table that causes the description of nested objects in the response schema to not get rendered.

@zulipbot zulipbot added size: L and removed size: XL labels Jul 17, 2022
@akashaviator akashaviator force-pushed the realm_presence branch 6 times, most recently from 496cb66 to e96892c Compare July 18, 2022 20:31
@akashaviator akashaviator changed the title [WIP]: Document /realm/presence API endpoint. api: Document /realm/presence API endpoint. Jul 18, 2022
@akashaviator akashaviator marked this pull request as ready for review July 18, 2022 20:32
@akashaviator akashaviator force-pushed the realm_presence branch 3 times, most recently from bf875ee to ec2cf18 Compare July 19, 2022 08:50
@akashaviator akashaviator changed the title api: Document /realm/presence API endpoint. [WIP]api: Document /realm/presence API endpoint. Jul 28, 2022
@timabbott timabbott requested a review from laurynmm July 28, 2022 22:34
@akashaviator akashaviator changed the title [WIP]api: Document /realm/presence API endpoint. api: Document /realm/presence API endpoint. Jul 29, 2022
`render_table` calls itself recursively when it finds nested
`additionalProperties` (i.e. nested objects) in response schema,
to render their properties.

This fixes `render_table` to call `render_desc` along with
calling itself, to render the description of the nested
`additionalProperties` as well.
Copy link
Collaborator

@laurynmm laurynmm left a comment

Choose a reason for hiding this comment

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

@akashaviator - In general, this all looks good to me. I did notice that the aggregated key for the presence objects wasn't being documented, which led me to look at the other places that presence is documented.

And I found a bunch of interrelated things to fix, so I put up an updated version of this PR with those changes (#22641). Would you be up for looking at those updates / changes?

@timabbott
Copy link
Sponsor Member

Closing in favor of #22641, huge thanks for documenting this @akashaviator!

@timabbott timabbott closed this Aug 4, 2022
@akashaviator akashaviator deleted the realm_presence branch August 6, 2022 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants