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

Fix RealtimePresence.get #299

Merged
merged 8 commits into from
Mar 17, 2016
Merged

Fix RealtimePresence.get #299

merged 8 commits into from
Mar 17, 2016

Conversation

ricardopereira
Copy link
Contributor

@ricardopereira ricardopereira force-pushed the fix-presence-get branch 4 times, most recently from c790c20 to f029fc6 Compare March 15, 2016 18:03
@ricardopereira
Copy link
Contributor Author 

[18:40:48]: ▸ Failing tests:
[18:40:48]: ▸ -[ARTRealtimePresenceTest testEnterAndGet]
[18:40:48]: ▸ -[ARTRealtimePresenceTest testEnterClient]
[18:40:48]: ▸ -[ARTRealtimePresenceTest testEnterClient]
[18:40:48]: ▸ -[ARTRealtimePresenceTest testLeaveAndGet]
[18:40:48]: ▸ -[ARTRealtimePresenceTest testPresenceNoSideEffects]
[18:40:48]: ▸ -[ARTRealtimePresenceTest testPresenceWithData]
[18:40:48]: ▸ -[ARTRealtimePresenceTest testPresenceWithData]

One of the problems is related with the order of the members in the PresenceMap. Sometimes the first connected client is in second place on the map. I looked at the specs and there's nothing specifying how it should be ordered. Should it be ordered by clientId or respect the order they were added?

@ricardopereira
Copy link
Contributor Author

@mattheworiordan @tcard ^

@ricardopereira ricardopereira force-pushed the fix-presence-get branch 2 times, most recently from 3ce8d63 to 957b709 Compare March 16, 2016 08:58
@tcard
Copy link
Contributor

tcard commented Mar 16, 2016

@ricardopereira I'd say we should leave it unspecified. I don't see how the order matters.

@ricardopereira
Copy link
Contributor Author

@tcard Looks good?

tcard
tcard reviewed Mar 16, 2016
View reviewed changes
Source/ARTChannel.h
@@ -33,8 +33,8 @@ ART_ASSUME_NONNULL_BEGIN
- (void)publish:(__GENERIC(NSArray, ARTMessage *) *)messages;
- (void)publish:(__GENERIC(NSArray, ARTMessage *) *)messages callback:(art_nullable void (^)(ARTErrorInfo *__art_nullable error))callback;

- (void)history:(void(^)(__GENERIC(ARTPaginatedResult, ARTMessage *) *__art_nullable result, NSError *__art_nullable error))callback;
- (BOOL)history:(art_nullable ARTDataQuery *)query callback:(void(^)(__GENERIC(ARTPaginatedResult, ARTMessage *) *__art_nullable result, NSError *__art_nullable error))callback error:(NSError *__art_nullable *__art_nullable)errorPtr;
- (void)history:(void(^)(__GENERIC(ARTPaginatedResult, ARTMessage *) *__art_nullable result, ARTErrorInfo *__art_nullable error))callback;
Copy link
Contributor

Choose a reason for hiding this comment

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

This change needs to be done in https://github.com/ably/docs as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You mean the code examples?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Issue #304

@ricardopereira ricardopereira mentioned this pull request Mar 17, 2016
Merged
@ricardopereira
Fix: undefined symbols __ObjC.ARTRealtimePresenceQuery
c7a4eb3 
 - Linker error: "Undefined symbols for architecture x86_64: _OBJC_CLASS_$_ARTRealtimePresenceQuery", referenced from: type metadata accessor for __ObjC.ARTRealtimePresenceQuery in RealtimeClientPresence.o ld: symbol(s) not found for architecture x86_64"
@ricardopereira
Fix: Presence query waitForSync should be true by default
2425954
@ricardopereira
Fix: Presence.get with no query argument should use the ARTRealtimePr…
ed391ff 
…esenceQuery be default
@ricardopereira
Replace NSError with ARTErrorInfo from RealtimePresence
f9a87cf 
 - `attach` callback uses an ARTErrorInfo instead of NSError.
@ricardopereira
Fix: RealtimePresence.get should return an array of PresenceMessage
e4ba4c8
@ricardopereira
Remove unused class ARTIndirectCancellable
ea6ef89
@ricardopereira
Remove ARTPresenceLast
f39fab6
ricardopereira added a commit that referenced this pull request Mar 17, 2016
@ricardopereira
Merge pull request #299 from ably/fix-presence-get
50cd090 
Fix: RealtimePresence.get
@ricardopereira ricardopereira merged commit 50cd090 into master Mar 17, 2016
@ricardopereira ricardopereira deleted the fix-presence-get branch March 17, 2016 20:53
maratal pushed a commit that referenced this pull request Jul 19, 2023
@SimonWoolf
Merge pull request #299 from ably/transportparams
2b10990 
Add transportParams clientOption to features spec
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.
2 participants
@ricardopereira @tcard