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

[SDK-3560] Add mandatory version param to Rest.request #1231

Merged

Conversation

lawrence-forooghian
Copy link
Collaborator

@lawrence-forooghian lawrence-forooghian commented Apr 27, 2023

Resolves #1225. See commit messages for more details.

@lawrence-forooghian lawrence-forooghian changed the base branch from main to integration/v2 April 27, 2023 20:21
@github-actions github-actions bot temporarily deployed to staging/pull/1231/features April 27, 2023 20:22 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1231/bundle-report April 27, 2023 20:23 Inactive
@lawrence-forooghian lawrence-forooghian force-pushed the 1225-add-version-param-to-Rest.request branch from fb34429 to 6354d9d Compare April 27, 2023 20:24
@github-actions github-actions bot temporarily deployed to staging/pull/1231/features April 27, 2023 20:24 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1231/bundle-report April 27, 2023 20:25 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1231/typedoc April 27, 2023 20:26 Inactive
@lawrence-forooghian lawrence-forooghian force-pushed the 1225-add-version-param-to-Rest.request branch from 6354d9d to d1c9ca9 Compare April 27, 2023 20:28
@github-actions github-actions bot temporarily deployed to staging/pull/1231/features April 27, 2023 20:29 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1231/bundle-report April 27, 2023 20:30 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1231/typedoc April 27, 2023 20:30 Inactive
@lawrence-forooghian lawrence-forooghian force-pushed the 1225-add-version-param-to-Rest.request branch from d1c9ca9 to f828bc6 Compare May 1, 2023 12:57
@github-actions github-actions bot temporarily deployed to staging/pull/1231/features May 1, 2023 12:58 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1231/bundle-report May 1, 2023 12:59 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1231/typedoc May 1, 2023 13:00 Inactive
@lawrence-forooghian lawrence-forooghian force-pushed the 1225-add-version-param-to-Rest.request branch from f828bc6 to 53c94d1 Compare May 1, 2023 14:48
@github-actions github-actions bot temporarily deployed to staging/pull/1231/features May 1, 2023 14:48 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1231/bundle-report May 1, 2023 14:50 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1231/typedoc May 1, 2023 14:50 Inactive
@lawrence-forooghian lawrence-forooghian force-pushed the 1225-add-version-param-to-Rest.request branch from 53c94d1 to 5e6accb Compare May 1, 2023 18:11
@github-actions github-actions bot temporarily deployed to staging/pull/1231/features May 1, 2023 18:11 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1231/typedoc May 1, 2023 18:13 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1231/bundle-report May 1, 2023 18:13 Inactive
@lawrence-forooghian lawrence-forooghian force-pushed the 1225-add-version-param-to-Rest.request branch from 5e6accb to fe44cd3 Compare May 1, 2023 19:05
@github-actions github-actions bot temporarily deployed to staging/pull/1231/features May 1, 2023 19:06 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1231/typedoc May 1, 2023 19:07 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1231/bundle-report May 1, 2023 19:07 Inactive
@lawrence-forooghian lawrence-forooghian force-pushed the 1225-add-version-param-to-Rest.request branch from fe44cd3 to 1e11659 Compare May 1, 2023 19:09
@github-actions github-actions bot temporarily deployed to staging/pull/1231/features May 1, 2023 19:09 Inactive
@lawrence-forooghian lawrence-forooghian force-pushed the 1225-add-version-param-to-Rest.request branch from 1e11659 to bcf8711 Compare May 1, 2023 19:09
@github-actions github-actions bot temporarily deployed to staging/pull/1231/features May 1, 2023 19:10 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1231/bundle-report May 1, 2023 19:11 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1231/typedoc May 1, 2023 19:11 Inactive
I intend to add further optional params and this will allow us to
disambiguate them.
As described by RSC19f1 in ably/specification at commit 505238c.

The docstrings are taken from ably/sdk-api-reference at commit 1e0776e.
The corresponding PR [1] has been approved by Owen but is still awaiting
tech writer approval. So we might need to tweak these docstrings later,
but I don’t think that needs to block us on this feature.

Approach for mocking HTTP request handling copied from http.test.js.

Other than the added test, the test changes are minimal (just adding an
extra argument to a few calls) but Prettier has decided to do some
re-indentation that makes them look larger than they are.

Resolves #1225.

[1] ably/sdk-api-reference#32
Copy link
Member

@owenpearson owenpearson left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@lawrence-forooghian lawrence-forooghian merged commit 5c2597e into integration/v2 May 3, 2023
@lawrence-forooghian lawrence-forooghian deleted the 1225-add-version-param-to-Rest.request branch May 3, 2023 14:29
@hayleynewton hayleynewton changed the title Add mandatory version param to Rest.request [SDK-3560] Add mandatory version param to Rest.request Jul 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants