Skip to content

Tweak spec for Rest.request’s version param - #145

Merged
lawrence-forooghian merged 2 commits into
integration/3.0.0from
tweak-spec-for-request-version-param
May 2, 2023
Merged

Tweak spec for Rest.request’s version param#145
lawrence-forooghian merged 2 commits into
integration/3.0.0from
tweak-spec-for-request-version-param

Conversation

@lawrence-forooghian

@lawrence-forooghianlawrence-forooghian commented May 1, 2023

Copy link
Copy Markdown
Contributor

Note: This is based on top of PR #143 — please review that one first.

This tweaks the type of this param and describes how the library is meant to use it. See commit messages for more details.

The related PR ably/sdk-api-reference#32 adds documentation for this parameter.

@github-actions
github-actionsBot temporarily deployed to staging/pull/145 May 1, 2023 18:44 Inactive
@lawrence-forooghian
lawrence-forooghian marked this pull request as ready for review May 1, 2023 18:44
CSV2a says that the protocol version is an integer.
@lawrence-forooghian
lawrence-forooghianforce-pushed the tweak-spec-for-request-version-param branch from 688fe9d to c184af0CompareMay 1, 2023 18:56
@github-actions
github-actionsBot temporarily deployed to staging/pull/145 May 1, 2023 18:57 Inactive
lawrence-forooghian added a commit to ably/ably-js that referenced this pull request May 1, 2023
As described by RSC19f1.
Approach for mocking HTTP request handling copied from http.test.js.
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.
Waiting on:
- ably/specification#143
- ably/specification#145
- ably/sdk-api-reference#32
lawrence-forooghian added a commit to ably/ably-js that referenced this pull request May 1, 2023
As described by RSC19f1.
Approach for mocking HTTP request handling copied from http.test.js.
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.
Waiting on:
- ably/specification#143
- ably/specification#145
- ably/sdk-api-reference#32
lawrence-forooghian added a commit to ably/ably-js that referenced this pull request May 1, 2023
As described by RSC19f1.
Approach for mocking HTTP request handling copied from http.test.js.
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.
Waiting on:
- ably/specification#143
- ably/specification#145
- ably/sdk-api-reference#32Resolves#1225.

@owenpearsonowenpearson left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks

Base automatically changed from reintroduce-breaking-api-changes to integration/3.0.0May 2, 2023 12:02
There’s still a question of how this method is meant to behave if the user also
specifies an `X-Ably-Header` header in the `headers` parameter. But it’s a
question that equally applied before adding the `version` parameter, so I’ve
captured it in a separate issue #144.
Same comment as 69e1973 re needing to update "replaced in" spec version before
merging integration branch into `main` applies here too.
@lawrence-forooghian
lawrence-forooghianforce-pushed the tweak-spec-for-request-version-param branch from c184af0 to c653156CompareMay 2, 2023 12:05
@lawrence-forooghian
lawrence-forooghian merged commit 505238c into integration/3.0.0May 2, 2023
@lawrence-forooghian
lawrence-forooghian deleted the tweak-spec-for-request-version-param branch May 2, 2023 12:11
ttypic pushed a commit that referenced this pull request Mar 6, 2026
Tweak spec for `Rest.request`’s `version` param
ttypic pushed a commit that referenced this pull request Mar 6, 2026
Tweak spec for `Rest.request`’s `version` param
Sign up for freeto 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

@lawrence-forooghian@SimonWoolf@owenpearson