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

Updates to API template #982

Merged
merged 2 commits into from
Jul 11, 2019
Merged

Updates to API template #982

merged 2 commits into from
Jul 11, 2019

Conversation

lcawl
Copy link
Contributor

@lcawl lcawl commented Jul 11, 2019

Related to #937

This PR updates the API reference template to change the location of the Required/Optional parameter qualifier (hopefully for the last time!).

I also modified the comment about the definitions list, since I think we should have some leeway for whether we create separate pages for all objects or not. i.e. For small objects it might annoy users to be sent to a separate page.

@lcawl lcawl requested review from KOTungseth and jrodewig July 11, 2019 17:11
* Include default values as the last sentence of the first paragraph.
* Include a range of valid values, if applicable.
* If the parameter requires a specific delimiter for multiple values, say so.
* If the parameter supports wildcards, ditto.
* For objects or nested objects, link to a separate definition list.
* For large or nested objects, consider linking to a separate definition list.
Copy link
Contributor

Choose a reason for hiding this comment

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

Oohhh. Good catch.

@@ -46,13 +46,12 @@ Guidelines for parameter documentation
***************************************
* Use a definition list.
* End each definition with a period.
* Include the data type.
* Include whether the parameter is Optional or Required.
* Include the data type and whether the parameter is Optional or Required.
Copy link
Contributor

@jrodewig jrodewig Jul 11, 2019

Choose a reason for hiding this comment

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

Should we reverse the order here since the current template includes optional/required THEN data type?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, thanks!

Copy link
Contributor

@jrodewig jrodewig left a comment

Choose a reason for hiding this comment

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

One minor comment, but this is fine as-is. Thanks for iterating on this @lcawl. 🥇

@lcawl lcawl merged commit 594d0b2 into elastic:master Jul 11, 2019
@lcawl lcawl deleted the api-template branch July 11, 2019 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants