Rsa8e #487
Rsa8e #487
Conversation
|
|
||
| let mergedParams2 = rest.auth.mergeParams(tokenParams2) | ||
| expect(mergedParams2.ttl) == 25.0 | ||
| expect(mergedParams2.clientId).to(beNil()) |
There was a problem hiding this comment.
I am not really following the test code here. Thee test code is not testing the requestToken method here but instead you are testing private undocumented APIs. The point of the test is to "should not merge with the configured params and options but instead replace all corresponding values, even when @null@" yet this is not asserting that?
There was a problem hiding this comment.
Many thanks, I'll improve it.
| @@ -91,6 +91,7 @@ ART_ASSUME_NONNULL_BEGIN | |||
| - (NSString *)description; | |||
|
|
|||
| - (ARTAuthOptions *)mergeWith:(ARTAuthOptions *)precedenceOptions; | |||
There was a problem hiding this comment.
Is this function called from somewhere now?
There was a problem hiding this comment.
Yes, in authorise method.
There was a problem hiding this comment.
OK, I'll guess it will go away later as now authorize also replaces rather than merging.
|
@tcard thanks for feedback, please find time to review. |
|
LGTM |
As a step by step part of /~https://github.com/ably/docs/pull/113/files