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

Proper RSH8b and RSH3g2a behavior. #1847

Merged
merged 17 commits into from
Jan 18, 2024
Merged

Proper RSH8b and RSH3g2a behavior. #1847

merged 17 commits into from
Jan 18, 2024

Conversation

maratal
Copy link
Collaborator

@maratal maratal commented Dec 21, 2023

See #1842, #1253 and commit messages for context.

…n activate) and initialising `clientId` at the same moment (RSH8b).

Device's `id` and `secret` no longer are being generated lazily on the first call (RSH8a).
I didn't touch any threading code here on purpose, since this PR is already complex enough.
Many tests relied on device details were there by default, so the only way to supply the details there (without proper state machine activation sequence) is to artificially call the `setupLocalDevice()`.
@github-actions github-actions bot temporarily deployed to staging/pull/1847/features December 21, 2023 13:46 Inactive
@maratal maratal requested review from sacOO7 and ttypic December 21, 2023 13:49
@github-actions github-actions bot temporarily deployed to staging/pull/1847/jazzydoc December 21, 2023 13:50 Inactive
Source/ARTLocalDevice.m Outdated Show resolved Hide resolved
@github-actions github-actions bot temporarily deployed to staging/pull/1847/features December 21, 2023 19:00 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1847/jazzydoc December 21, 2023 19:04 Inactive
Copy link
Contributor

@ttypic ttypic left a comment

Choose a reason for hiding this comment

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

LGTM

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.

Do the tests not need updating (or new ones adding) for the new behaviours that you've implemented? There are existing tests that claim to test RSH8b and RSH3g2a but presumably they're inadequate given that those spec points weren't implemented.

Source/ARTLocalDevice.m Outdated Show resolved Hide resolved
Source/ARTLocalDevice.m Outdated Show resolved Hide resolved
Source/ARTLocalDevice.m Outdated Show resolved Hide resolved
Source/ARTPushActivationState.m Outdated Show resolved Hide resolved
Source/ARTLocalDevice.m Outdated Show resolved Hide resolved
Source/ARTRest.m Outdated Show resolved Hide resolved
Source/ARTRest.m Outdated Show resolved Hide resolved
@github-actions github-actions bot temporarily deployed to staging/pull/1847/features January 4, 2024 18:20 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1847/features January 4, 2024 18:21 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1847/jazzydoc January 4, 2024 18:24 Inactive
@maratal maratal force-pushed the fix/1177-RSH8b-RSH3g2a branch from 6c6f0fb to 9277ab0 Compare January 4, 2024 19:06
@github-actions github-actions bot temporarily deployed to staging/pull/1847/features January 4, 2024 19:06 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1847/jazzydoc January 4, 2024 19:10 Inactive
@maratal maratal force-pushed the fix/1177-RSH8b-RSH3g2a branch from 9277ab0 to 12521f8 Compare January 4, 2024 19:18
@github-actions github-actions bot temporarily deployed to staging/pull/1847/features January 4, 2024 19:19 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1847/jazzydoc January 4, 2024 19:23 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1847/features January 7, 2024 13:22 Inactive
@maratal maratal mentioned this pull request Jan 13, 2024
@maratal maratal force-pushed the fix/1177-RSH8b-RSH3g2a branch from f934274 to 7159d42 Compare January 14, 2024 19:57
@github-actions github-actions bot temporarily deployed to staging/pull/1847/features January 14, 2024 19:57 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1847/jazzydoc January 14, 2024 20:00 Inactive
@maratal maratal force-pushed the fix/1177-RSH8b-RSH3g2a branch from 7159d42 to 1873d81 Compare January 14, 2024 20:43
@github-actions github-actions bot temporarily deployed to staging/pull/1847/features January 14, 2024 20:43 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1847/jazzydoc January 14, 2024 20:47 Inactive
Source/ARTLocalDevice.m Show resolved Hide resolved
Source/ARTLocalDevice.m Outdated Show resolved Hide resolved
Source/ARTPushActivationState.m Outdated Show resolved Hide resolved
@github-actions github-actions bot temporarily deployed to staging/pull/1847/features January 17, 2024 21:09 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1847/jazzydoc January 17, 2024 21:15 Inactive
Source/ARTLocalDevice.m Outdated Show resolved Hide resolved
Source/ARTLocalDevice.m Outdated Show resolved Hide resolved
Co-authored-by: Lawrence Forooghian <[email protected]>
@maratal maratal merged commit 4ff1202 into main Jan 18, 2024
7 checks passed
@maratal maratal deleted the fix/1177-RSH8b-RSH3g2a branch January 18, 2024 19:05
maratal added a commit to ably/specification that referenced this pull request Feb 4, 2024
…y/ably-cocoa#1847, where a new device ID is generated as soon as the previous ones are cleared, so that id is never null.
maratal added a commit to ably/specification that referenced this pull request Feb 13, 2024
…y/ably-cocoa#1847, where a new device ID is generated as soon as the previous ones are cleared, so that id is never null.
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.

3 participants