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

DeviceDetails.id (PCD2) should be nullable. #180

Open
maratal opened this issue Jan 16, 2024 · 7 comments · May be fixed by #181
Open

DeviceDetails.id (PCD2) should be nullable. #180

maratal opened this issue Jan 16, 2024 · 7 comments · May be fixed by #181

Comments

@maratal
Copy link
Collaborator

maratal commented Jan 16, 2024

Currently it is declared as non-nil for some reason:

class DeviceDetails: // PCD*
  id: String // PCD2

which leads to the bunch of problems and inconsistencies in the SDK(s) implementations.

Copy link

sync-by-unito bot commented Jan 16, 2024

➤ Automation for Jira commented:

The link to the corresponding Jira issue is https://ably.atlassian.net/browse/SDK-4038

maratal added a commit to ably/ably-cocoa that referenced this issue Jan 17, 2024
@lawrence-forooghian
Copy link
Collaborator

Alternatively we can update the spec so that it reflects the behaviour implemented in ably/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
Copy link
Collaborator Author

maratal commented Jan 18, 2024

Alternatively we can update the spec so that it reflects the behaviour implemented in ably/ably-cocoa#1847, where a new device ID is generated as soon as the previous ones are cleared, so that id is never null.

It's an idea. But I think it's better to eventually make nullable id nevertheless. And schedule this change together with the v2 protocol. WDYT?

@lawrence-forooghian
Copy link
Collaborator

But I think it's better to eventually make nullable id nevertheless.

Why do you think it better?

And schedule this change together with the v2 protocol. WDYT?

What's the relation with the v2 protocol?

@maratal
Copy link
Collaborator Author

maratal commented Jan 18, 2024

Why do you think it better?

Hmm, good question. At least it's nullable in other SDKs already.

What's the relation with the v2 protocol?

I mean v2 release is a breaking change and release of another breaking change together would make sense I think.

@lawrence-forooghian
Copy link
Collaborator

lawrence-forooghian commented Jan 22, 2024

At least it's nullable in other SDKs already.

By "other SDKs" do you mean just ably-java (which is currently the only other SDK to have a LocalDevice), or are there others? And in the case of ably-java I'm not really sure what it means to say that it's "nullable". Sure, the SDK sets it to null sometimes, but there's no language-level concept of nullability, and there's nothing in the documentation to suggest to the user that they can expect it to be null.

I mean v2 release is a breaking change

The changes introduced by version 2.0.0 of the specification, which is the version that switched to using protocol v2, do not include any breaking API changes.

@maratal
Copy link
Collaborator Author

maratal commented Jan 22, 2024

The changes introduced by version 2.0.0 of the specification, which is the version that switched to using protocol v2, do not include any breaking API changes.

You are right, the one that was deleted is marked as deprecated. Ok, will change the specs then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants