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-3561] New batch specs #139

Merged
merged 4 commits into from
Jul 24, 2023
Merged

[SDK-3561] New batch specs #139

merged 4 commits into from
Jul 24, 2023

Conversation

owenpearson
Copy link
Member

@owenpearson owenpearson commented Apr 28, 2023

Resolves #140

Doing this now so that we can publish this as spec v2.1 before working on spec v3

Adds new specs for batch REST methods, using the new batch response format via the newBatchResponse query param.

For further information, see:

@github-actions github-actions bot temporarily deployed to staging/pull/139 April 28, 2023 16:41 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/139 April 28, 2023 16:42 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/139 April 28, 2023 16:48 Inactive
Copy link
Collaborator

@lawrence-forooghian lawrence-forooghian left a comment

Choose a reason for hiding this comment

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

As well as the comments below, I also wanted to mention that we need to write documentation comments for these new APIs and open a PR into https://github.com/ably/sdk-api-reference. The process described by EdX DR 5.4 isn't quite clear about when this should happen, but I’d suggest that it would be best to do it alongside the spec PR (i.e. now), so that reviewers of the spec PR can also understand how the new functionality is meant to be used by users.

textile/features.textile Outdated Show resolved Hide resolved
textile/features.textile Outdated Show resolved Hide resolved
textile/features.textile Outdated Show resolved Hide resolved
textile/features.textile Outdated Show resolved Hide resolved
textile/features.textile Outdated Show resolved Hide resolved
textile/features.textile Outdated Show resolved Hide resolved
textile/features.textile Outdated Show resolved Hide resolved
textile/features.textile Outdated Show resolved Hide resolved
textile/features.textile Outdated Show resolved Hide resolved
textile/features.textile Outdated Show resolved Hide resolved
@github-actions github-actions bot temporarily deployed to staging/pull/139 May 2, 2023 08:16 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/139 May 2, 2023 08:43 Inactive
Copy link
Member

@SimonWoolf SimonWoolf left a comment

Choose a reason for hiding this comment

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

Looks good, couple comments

textile/features.textile Outdated Show resolved Hide resolved
textile/features.textile Outdated Show resolved Hide resolved
textile/features.textile Outdated Show resolved Hide resolved
textile/features.textile Outdated Show resolved Hide resolved
textile/features.textile Outdated Show resolved Hide resolved
textile/features.textile Outdated Show resolved Hide resolved
@github-actions github-actions bot temporarily deployed to staging/pull/139 May 2, 2023 13:59 Inactive
textile/features.textile Outdated Show resolved Hide resolved
textile/features.textile Outdated Show resolved Hide resolved
@github-actions github-actions bot temporarily deployed to staging/pull/139 May 4, 2023 10:42 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/139 May 4, 2023 10:46 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/139 May 4, 2023 14:13 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/139 May 4, 2023 15:15 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/139 May 4, 2023 16:16 Inactive
textile/features.textile Outdated Show resolved Hide resolved
textile/features.textile Outdated Show resolved Hide resolved
textile/features.textile Outdated Show resolved Hide resolved
textile/features.textile Outdated Show resolved Hide resolved
textile/features.textile Outdated Show resolved Hide resolved
@github-actions github-actions bot temporarily deployed to staging/pull/139 May 17, 2023 08:57 Inactive
Copy link
Collaborator

@lawrence-forooghian lawrence-forooghian left a comment

Choose a reason for hiding this comment

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

Looks good, one outstanding question about the messageId response property, but feel free to resolve and merge if you consider it a non-blocker.

owenpearson and others added 3 commits July 20, 2023 13:30
These were never implemented in any SDK so is safe to remove without
deprectation
@github-actions github-actions bot temporarily deployed to staging/pull/139 July 20, 2023 16:31 Inactive
Co-authored-by: Lawrence Forooghian <[email protected]>
@lawrence-forooghian lawrence-forooghian merged commit 85f1a39 into main Jul 24, 2023
2 checks passed
@lawrence-forooghian lawrence-forooghian deleted the new-batch-specs branch July 24, 2023 13:07
@hayleynewton hayleynewton changed the title New batch specs [SDK-3561] New batch specs 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.

Add SDK APIs for new batch REST API format
3 participants