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.

Sign up for GitHub

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

Jump to bottom

[WIP] Wrapper sdk proxy #2014

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

[WIP] Wrapper sdk proxy #2014

wants to merge 17 commits into from

Conversation

lawrence-forooghian
Copy link
Collaborator

No description provided.

@coderabbitai coderabbitai
Copy link

coderabbitai bot commented Jan 15, 2025

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions github-actions bot temporarily deployed to staging/pull/2014/features January 15, 2025 12:51 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/2014/jazzydoc January 15, 2025 12:54 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/2014/features January 15, 2025 14:09 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/2014/jazzydoc January 15, 2025 14:12 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/2014/features January 15, 2025 14:30 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/2014/jazzydoc January 15, 2025 14:34 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/2014/features January 15, 2025 14:51 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/2014/jazzydoc January 15, 2025 14:54 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/2014/features January 15, 2025 17:39 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/2014/jazzydoc January 15, 2025 17:41 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/2014/features January 15, 2025 19:40 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/2014/jazzydoc January 15, 2025 19:43 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/2014/features January 15, 2025 20:01 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/2014/jazzydoc January 15, 2025 20:03 Inactive
@lawrence-forooghian
Revert 26d9bf7
e216ae5 
That change, which moved the `presence` property from the
ARTRealtimeChannel class to ARTRealtimeChannelProtocol, changed the type
of the property from ARTRealtimePresence to
id<ARTRealtimePresenceProtocol>. This is a breaking change for anybody
who was relying on the property returning the concrete type, one such
example being our Asset Tracking SDK. So, we undo it.

The change made in 26d9bf7 was triggered by us noticing, whilst working
on the Chat SDK — which does its best to avoid referring to ably-cocoa's
concrete types — that when you have an ably-cocoa concrete type
ConcreteA which implements a protocol ProtocolA, and where ConcreteA has
a public method or property `someMethod` which returns an instance of
another concrete type ConcreteB which happens to implement a protocol
ConcreteB, then `someMethod` is always defined directly on ConcreteA,
not on ProtocolA. We wondered "why can't these methods be on ProtocolA
instead?", hence 26d9bf7. But the first reason is that we can't because
in order to do this it makes sense for you to also change the the return
type of `someMethod` from ConcreteB to ProtocolB, which is a breaking
change as we've seen here. But also I'm not sure whether it'd work even
if we accepted the breaking change; for example, I don't know if you can
mark a protocol as Sendable in Objective-C (haven't looked into it since
we currently have no intention of making the breaking change, but one to
investigate if we consider it in the future).

Resolves #2003.
@lawrence-forooghian
Remove initializers out of client protocols
91fa3ea 
Preparation for introducing a new class that conforms to these protocols
but which won't have these initializers. Let's use these protocols just
to list the functionality provided by an already-initialized client.

(TODO is this a breaking change)
@lawrence-forooghian
remove unavailable initializers from public protocols
c4bd450
@lawrence-forooghian
Add some missing nullability annotations
c5caf00
@lawrence-forooghian
remove public protocol conformances from internal classes
0f2aee8 
this is so that we can do things like move `channels` into them (the
types currently clash; e.g. one is `ChannelsInternal` and one is
`Channels`), and it's just unnecessarily restrictive. perhaps there's
some undocumented characteristic of the `Internal` classes that i'm
violating, but oh well

(i still remain confused about the purpose of the Internal classes)

(TODO not sure if, post-removal of attempting to use interfaces for
properties, this is still needed)
@lawrence-forooghian
Remove redundant property
af3318f 
It's already declared in ARTChannelProtocol.
@lawrence-forooghian
Make history: available through ARTRestPresenceProtocol
3806faa
@lawrence-forooghian
Add method for creating wrapper SDK proxy client
3e39f8c 
TODO same for REST?
@lawrence-forooghian
(TODO Incorporate) make agents consistent with client options
be15fcf
@lawrence-forooghian
remove ARTHttpAuthenticatedExecutor
68a978e 
it's unused (i.e. nobody takes one as a dependency)
@lawrence-forooghian
Remove ARTRestInternal's conformance to ARTHTTPExecutor
2823487 
Nobody makes use of this conformance.
@lawrence-forooghian
insert wrapper SDK agents in -request
2db4ec0 
note that this justifies our earlier making of the internal not conform
to the public protocol

note this provides groundwork for allowing other REST-triggering things
to also supply wrapper SDK agents, but that we're currently passing nil
for wrapperSDKAgents param; will address that piecemeal later

TODO test
@lawrence-forooghian
add wrapper agents to -first and -next of -request's return value
381e21d 
TODO test
@github-actions github-actions bot temporarily deployed to staging/pull/2014/features January 16, 2025 19:15 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/2014/jazzydoc January 16, 2025 19:18 Inactive
@lawrence-forooghian
add proxy for channel
113d452 
this will allow us to insert the agent parameter (no functional changes
in this commit)

TODO this doesn't cover the channel's other properties

TODO reuse existing instance
@lawrence-forooghian
Add NSCopying support to ART*ChannelOptions
7410363 
Will use in an upcoming commit, but it's also handy for users of the SDK
(e.g. I had already found myself wanting it in Chat, when wishing to
create a modified version of an options object recevied as an argument).

TODO test
@github-actions github-actions bot temporarily deployed to staging/pull/2014/features January 16, 2025 19:37 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/2014/jazzydoc January 16, 2025 19:38 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@lawrence-forooghian