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

docs: clean up WIP sections for sequence page #2994

Merged
merged 1 commit into from
Jun 1, 2019

Conversation

b-admike
Copy link
Contributor

Fixes #2100

Checklist

👉 Read and sign the CLA (Contributor License Agreement) 👈

  • npm test passes on your machine
  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style guide
  • API Documentation in code was updated
  • Documentation in /docs/site was updated
  • Affected artifact templates in packages/cli were updated
  • Affected example projects in examples/* were updated

👉 Check out how to submit a PR 👈

@b-admike b-admike force-pushed the docs/sequence-cleanup branch 2 times, most recently from 369b9e2 to 3c5628f Compare May 31, 2019 17:40
@b-admike b-admike marked this pull request as ready for review May 31, 2019 17:42
@b-admike b-admike requested a review from raymondfeng as a code owner May 31, 2019 17:42

- Try and use existing actions
- Implement your own version of built in actions
- Publish reusable actions to npm
Copy link
Contributor Author

@b-admike b-admike May 31, 2019

Choose a reason for hiding this comment

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

see #3021 for Publish reusable actions to npm section.

@b-admike b-admike changed the title docs: add query string section for sequences docs: clean up WIP sections for sequence page May 31, 2019
Copy link
Contributor

@nabdelgadir nabdelgadir left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Nitpick: can you un-indent the code blocks? (feel free to ignore)

docs/site/Sequence.md Outdated Show resolved Hide resolved
@b-admike b-admike force-pushed the docs/sequence-cleanup branch 3 times, most recently from 58eabcd to e954ef0 Compare May 31, 2019 19:31
@b-admike b-admike force-pushed the docs/sequence-cleanup branch 2 times, most recently from 141c450 to 1d1ad0d Compare June 1, 2019 01:36
@b-admike b-admike self-assigned this Jun 1, 2019
Copy link
Contributor

@jannyHou jannyHou left a comment

Choose a reason for hiding this comment

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

@b-admike A few nitpicks otherwise LGTM :shipit: Good content!

argument is typically decorated by @param(). We've made multiple shortcuts
available to the `@param()` decorator in the form of
`@param.<http_source>.<OAI_primitive_type>`. Using this notation, query string
parameters can be described as `@param.query.<OAI_primitive_type>`. Here is an
Copy link
Contributor

Choose a reason for hiding this comment

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

typo? I think you mean @param.query.string =p

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch :-). Fixed!


You can also specify a parameter which is an object value encoded as a JSON
string or in multiple nested keys. For a JSON string, a sample value would be
`location={"lang": 23.414, "lat": -98.1515}`. For the same `location` object, it
Copy link
Contributor

Choose a reason for hiding this comment

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

A question: is it meant to be the string value get from the HTTP client?

If we are showing an example value from in the API explorer input field, probably wrap key&value with double quotes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually I took this straight from API explorer and I think they're not wrapped because we define lat/long as number values.

@b-admike b-admike force-pushed the docs/sequence-cleanup branch from 1d1ad0d to 634291b Compare June 1, 2019 02:30
@b-admike b-admike force-pushed the docs/sequence-cleanup branch from 634291b to 2c63955 Compare June 1, 2019 02:49
@b-admike b-admike merged commit 1f5f1e2 into master Jun 1, 2019
@delete-merged-branch delete-merged-branch bot deleted the docs/sequence-cleanup branch June 1, 2019 03:14
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.

[Docs] Update WIP section in Sequence page
5 participants