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

Stats function implementation #324

Draft
wants to merge 52 commits into
base: main
Choose a base branch
from
Draft

Stats function implementation #324

wants to merge 52 commits into from

Conversation

ikurek
Copy link
Contributor

@ikurek ikurek commented Feb 8, 2022

This is a rework of #283 because the previous PR was made from enhancement/formatting and the branch can't be merged. The logic had to be extended, since the solution presented in previous PR didn't work.

Original description from #283:

This PR includes full implementation of Rest.stats function for Flutter plugin. As stats is a slightly complicated and big object, this pull request might seem a bit huge but the scope is still just a map of a function.
Implemenation contains

  • New type definitions on Flutter side for code generation
  • New encoders for Java
  • New encoders for Swift/Objective-C
  • Function mappings
  • A paginated result viewer addition to example app

There were some differences between native implementations:

  • Structures were formed slightly differently. I created an issue in ably-java for this
  • Stats structure is not fully compatible with spec ably-java#733

The way stats params were received was a bit more structured in Objective C than Java. In Objective C there is a dedicated object for this, where it's a Param[] in Java. I have created a separate category in Objective C to build that object from the map sent from Dart side.

Additions from my side:

  • Removed unnecessary formatting and changes
  • Fixed issue with null handle being passed to iOS method
  • Implemented dart decoder for Stats objects

This pull request remains blocked untill ably-java#733 and ably-cocoa#1284 are resolved, so I'm leaving it as a draft

ikbalkaya and others added 30 commits February 8, 2022 18:20
@ikurek ikurek added enhancement New feature or improved functionality. blocked-by-ably-cocoa labels Feb 8, 2022
@ikurek ikurek self-assigned this Feb 8, 2022
@ikurek ikurek changed the base branch from main to feature/time-function February 8, 2022 17:36
@ikurek ikurek changed the title Stats function Stats function implementation Feb 8, 2022
@github-actions github-actions bot temporarily deployed to staging/pull/324/dartdoc February 8, 2022 17:38 Inactive
@ikurek ikurek mentioned this pull request Feb 8, 2022
@ikurek ikurek linked an issue Feb 9, 2022 that may be closed by this pull request
Base automatically changed from feature/time-function to main February 25, 2022 13:40
@QuintinWillison QuintinWillison added blocked-by-ably We can't proceed until something under our direct control, in a different codebase, happens. and removed blocked-by-ably-cocoa labels Dec 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked-by-ably We can't proceed until something under our direct control, in a different codebase, happens. enhancement New feature or improved functionality.
Development

Successfully merging this pull request may close these issues.

Implement stats Rest and Realtime clients
3 participants